config(customization): customization override #6545

Will-ShaoHua · 2025-02-10T21:17:30Z

Problem

https://github.com/aws/aws-toolkit-vscode/blob/master/packages/core/src/codewhisperer/util/customizationUtil.ts#L116-L130

The flaw of previous implementation won't override customization if users have selected one.

The team is asking

Ability to override regardless users are with a customization or not
Should respect users' selection afterward when users switch back to the customization they prefer
When ab service provides a new override customization arn, plugins should be able to do override again

// as long as there is a customization selected, override will not happen no matter what
 if (selectedCustomization && selectedCustomization.name !== '') {
        return selectedCustomization
    } else {
        const customizationFeature = FeatureConfigProvider.getFeature(Features.customizationArnOverride)
        const arnOverride = customizationFeature?.value.stringValue
        const customizationOverrideName = customizationFeature?.variation
        if (arnOverride === undefined) {
            return baseCustomization
        } else {
            return {
                arn: arnOverride,
                name: customizationOverrideName,
                description: baseCustomization.description,
            }
        }

Solution

Persist a value of "previous override arn" and do override when the provided customization arn is different than "previous override arn"

Treat all work as PUBLIC. Private feature/x branches will not be squash-merged at release time.
Your code changes must meet the guidelines in CONTRIBUTING.md.
License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

github-actions · 2025-02-10T21:17:45Z

This pull request modifies code in src/* but no tests were added/updated.
- Confirm whether tests should be added or ensure the PR description explains why tests are not required.

jpinkney-aws

Not really familiar with this feature but wanted to leave a few comments out of curiosity

jpinkney-aws · 2025-02-11T21:16:49Z

packages/core/src/test/amazonq/customizationUtil.test.ts


-    it('Returns AB customization', async function () {
-        sinon.stub(AuthUtil.instance, 'isValidEnterpriseSsoInUse').returns(true)
+    it(`setSelectedCsutomization should set correctly if override is false or not specified`, async function () {


nit:

Suggested change

it(`setSelectedCsutomization should set correctly if override is false or not specified`, async function () {

it(`setSelectedCustomization should set correctly if override is false or not specified`, async function () {

What is this test case trying to illustrate? It's not really clear from the title

so when override is true, it's saying the setCustomization is invoked by us and not by users. In such case, the customization we pass in the call setCustomization is from A/B service (not from users), and we will override users' own selection once for each customization arn if there is one provided by the service.

when override is false, it means the setCustomization is invoked by users, so we will respect to users' selection anyway

jpinkney-aws · 2025-02-11T21:17:04Z

packages/core/src/test/amazonq/customizationUtil.test.ts

+        assert.strictEqual(getSelectedCustomization().arn, 'QOO')
+    })
+
+    it(`setSelectedCsutomization should only do once for override per customization arn`, async function () {


nit:

Suggested change

it(`setSelectedCsutomization should only do once for override per customization arn`, async function () {

it(`setSelectedCustomization should only do once for override per customization arn`, async function () {

jpinkney-aws · 2025-02-11T21:27:28Z

packages/core/src/codewhisperer/util/customizationUtil.ts

-export const setSelectedCustomization = async (customization: Customization) => {
+/**
+ * @param customization customization to select
+ * @param isOverride if the API call is made from us (Q) but not users' intent, set isOverride to TRUE


API call is made from us (Q)

Do you mean from the extension side?

yea, i want to express it's us to override users' selection and it's not on their own intention to change such configuration

jpinkney-aws · 2025-02-11T21:35:00Z

packages/core/src/codewhisperer/util/customizationUtil.ts

    if (!AuthUtil.instance.isValidEnterpriseSsoInUse() || !AuthUtil.instance.conn) {
        return
    }
+    if (isOverride) {


Just for my own understanding -- this means that the server has received a non empty override customization arn and we want to check whether or not its a new arn. If the same arn is found then we're good to go since there's nothing to do. Otherwise override the customization?

Is that correct?

his means that the server has received a non empty override customization arn and we want to check whether or not its a new arn.

this is correct.

If the same arn is found then we're good to go since there's nothing to do. Otherwise override the customization?

yes and no. If the customization arn is same as the last one (which we had done override once), we do nothing because it's intrusive if we change users' selection every time when there is a non-null override customization arn. So we simply ignore the override request for the 2nd times onward

leigaol

Please polish your description in the PR. Thanks!

justinmk3 · 2025-02-12T19:32:37Z

Should we not mention this in the changelog?

Will-ShaoHua · 2025-02-12T19:34:58Z

Should we not mention this in the changelog?

@justinmk3 probably not, as it's targeting a subset of users and experiment IMO. (and obviously it's only targeting internal users for sure)

## Problem https://github.com/aws/aws-toolkit-vscode/blob/master/packages/core/src/codewhisperer/util/customizationUtil.ts#L116-L130 The flaw of previous implementation won't override customization if users have selected one. The team is asking 1. Ability to override regardless users are with a customization or not 2. Should respect users' selection afterward when users switch back to the customization they prefer 3. When ab service provides a new override customization arn, plugins should be able to do override again ``` // as long as there is a customization selected, override will not happen no matter what if (selectedCustomization && selectedCustomization.name !== '') { return selectedCustomization } else { const customizationFeature = FeatureConfigProvider.getFeature(Features.customizationArnOverride) const arnOverride = customizationFeature?.value.stringValue const customizationOverrideName = customizationFeature?.variation if (arnOverride === undefined) { return baseCustomization } else { return { arn: arnOverride, name: customizationOverrideName, description: baseCustomization.description, } } ``` ## Solution 1. Persist a value of "previous override arn" and do override when the provided customization arn is different than "previous override arn" --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). - License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

Will-ShaoHua added 3 commits February 10, 2025 10:48

getSlectedCustomization should has no side effect

b4b2258

selectCustomization if override is provided by the service

b5e72bf

customization override should only happen once for a specific override

b29b0cc

Will-ShaoHua added 5 commits February 11, 2025 11:13

patch

63d1928

Merge remote-tracking branch 'upstream/master' into custom

cf20d12

lint

0c6c339

t

85618b8

test

e2fdbc1

Will-ShaoHua marked this pull request as ready for review February 11, 2025 20:47

Will-ShaoHua requested review from a team as code owners February 11, 2025 20:47

Will-ShaoHua requested a review from andrewyuq February 11, 2025 21:02

jpinkney-aws reviewed Feb 11, 2025

View reviewed changes

Will-ShaoHua added 2 commits February 11, 2025 13:48

t

6b41c8a

Merge remote-tracking branch 'upstream/master' into custom

96aac5e

Will-ShaoHua requested review from jpinkney-aws and leigaol February 11, 2025 22:04

Will-ShaoHua changed the title ~~config(customization): customizatino override~~ config(customization): customization override Feb 11, 2025

leigaol approved these changes Feb 11, 2025

View reviewed changes

jpinkney-aws approved these changes Feb 12, 2025

View reviewed changes

jpinkney-aws merged commit 80dfe15 into aws:master Feb 12, 2025
25 of 26 checks passed

Will-ShaoHua deleted the custom branch February 12, 2025 01:17

	it(`setSelectedCsutomization should set correctly if override is false or not specified`, async function () {
	it(`setSelectedCustomization should set correctly if override is false or not specified`, async function () {

	it(`setSelectedCsutomization should only do once for override per customization arn`, async function () {
	it(`setSelectedCustomization should only do once for override per customization arn`, async function () {

config(customization): customization override #6545

config(customization): customization override #6545

Uh oh!

Conversation

Will-ShaoHua commented Feb 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

Uh oh!

github-actions bot commented Feb 10, 2025

Uh oh!

jpinkney-aws left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Will-ShaoHua Feb 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Will-ShaoHua Feb 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

leigaol left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

justinmk3 commented Feb 12, 2025

Uh oh!

Will-ShaoHua commented Feb 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Will-ShaoHua commented Feb 10, 2025 •

edited

Loading

Will-ShaoHua Feb 11, 2025 •

edited

Loading

Will-ShaoHua Feb 11, 2025 •

edited

Loading

Will-ShaoHua commented Feb 12, 2025 •

edited

Loading