Enable the duplicate ability for custom ai service

[phlpsong]

#479

[Jerry23011]

There is an issue that newly created service config change event not get subscribed since services in GlobalContext not updated. I would like to create another PR to fix this.

Just to make sure. Is this issue description related to the problem below: the model picker wouldn't update when I manually add more models.

[phlpsong]

There is an issue that newly created service config change event not get subscribed since services in GlobalContext not updated. I would like to create another PR to fix this.

Just to make sure. Is this issue description related to the problem below: the model picker wouldn't update when I manually add more models.

Yes, you are right!

It looks caused by below code, the services not changed after new service duplicated:

I will check and fix it later.

[Jerry23011]

Yes, you are right!

It looks caused by below code, the services not changed after new service duplicated:

I will check and fix it later.

Got it, then I have no further questions here

[Jerry23011]

LGTM

[phlpsong]

@Jerry23011 The issue should be fixed in latest code changes.

[Jerry23011]

It still doesn't seem to work here

[phlpsong]

@Jerry23011 Are you using the latest code? It works fine from my side.

[Jerry23011]

@Jerry23011 Are you using the latest code? It works fine from my side.

Yes I did, I also cleaned the build folder and cleared the local persistent data. Could be an issue on my end. Let's wait for someone else to test this.

[phlpsong]

Ok, I will check it later.

[Jerry23011]

Now it works. LGTM

[tisfeng]

I briefly tested it and found some problems:

1. When a copied custom AI service does not have a URL set, clicking the expand button does not respond. Normally, it should show an error message just like the original custom OpenAI service.

[tisfeng]

2. I am not sure if it is a side effect caused by copying custom AI services. The service list on the settings page cannot be dragged to modify the order. I also discovered before that changing the order of services would cause the copied custom AI services to disappear. Anyway, there are a lot of weird questions here.

iShot_2024-08-26_22.03.27.mp4

[tisfeng]

3. After copying a custom AI service, it is found that it only exists in the current window type list, such as fixed window, but cannot be seen in the mini window, which is unreasonable.

iShot_2024-08-26_22.12.09.mp4

[tisfeng]

4. I modified the custom AI service name that was copied, but the service name in the query window did not update.

Additionally, the issue #647 (comment) mentioned earlier also occurred here, where the Picker's model did not sync update. I haven't tried clearing the cache yet, will try later.

[tisfeng]

I cleared the Xcode cache, recompiled and ran it, and found that the Picker model synchronization issue seems to be resolved.

However, after configuring the copied custom AI service, when I clicked the expand button, it still did not perform the expected query.

[phlpsong]

Thanks for your input.

For 1 & 2, they should be bugs that need to fix later.

For 3, as far as I am concerned, current behavior looks more reasonable than copy that into all window type.

Do you think we need to keep all service the same in different window types? If so, I think it's fine to duplicate one into all window type?

For 4 I will confirm it later, based on my testing before this should not exist anymore.

[tisfeng]

For 3, as far as I am concerned, current behavior looks more reasonable than copy that into all window type.

Do you think we need to keep all service the same in different window types? If so, I think it's fine to duplicate one into all window type?

Yes, currently I think it's ok to treat the copied service as a separate service type, so it should be able to show up in other window types as well, it's just better to set the default off (enabled = false) at this point.

Currently all window types use the same service, e.g. changing the OpenAI service configuration in a mini window also affects the fixed window, which is probably not well designed, but I don't know how to improve it, and the code is a mess 😥

[phlpsong]

OK, will fix them later.

[phlpsong]

Looks like no completion executed when no endpoint configured, that should be the root cause of issue 1, but not sure if this fix(fix: endpoint empty error result issue) is right, please kindly let me know what your thoughts.

[tisfeng]

Looks like no completion executed when no endpoint configured, that should be the root cause of issue 1, but not sure if this fix(fix: endpoint empty error result issue) is right, please kindly let me know what your thoughts.

No, this should be a legacy bug caused by the default value of isStreamFinished being YES. We should set it to NO before the stream request starts. I have fixed it in 7621e9e .

Previously, I intentionally did not add completion(result, error) here, but after thinking about it, the completion should be called even if isStreamFinished = true.

[tisfeng]

The model does not change with the Picker.

The model only updates when I refresh this Azure OpenAI service, for example, by turning it on or off.

[phlpsong]

Yes, it has some issues in this part, I will try to fix it later.

[phlpsong]

Please kindly continue to review this PR, much appreciate it.

[tisfeng]

When clicking to query, the custom AI service copied is missing a loading animation, please compare and check.

iShot_2024-09-08_10.04.09.mp4

[phlpsong]

I noticed this odd behavior too yesterday, but not found the root cause. I will check it today.

[tisfeng]

I simply debugged and found that the result that updates the data result.isLoading = YES; and the resultView that updates the loading animation [resultView updateLoadingAnimation] are not the same object, which should be the root of the problem.

I haven't looked at the code implementation yet, but I think the copied custom AI service should use an independent result, rather than using the same result as the custom AI service. Otherwise, it will be impossible to control the display of data through the result, such as the loading animation.

[phlpsong]

Thanks for the info above, the root cause is the index logic in BaseQueryViewController, should use type#Id as key.

[tisfeng]

初步测试下来，功能上暂时没问题了。 代码我还没仔细看，你们先看一下。

我准备先发一个简单修复版本，这个复制自定义 AI 功能稍后和自定义 prompt 一起上。

[tisfeng]

This code looks good! Let's merge it 😃

I just renamed serviceTypeWithIdIfHave to serviceTypeWithUniqueIdentifier.