Add waiting time for batch translation using GPT

[lyk7539511]

Related issue

460

Feature

Added batchTaskDelayGPT: number to specifically control the batch translation waiting time when using the GPT engine. OpenAI officially limits API calls to three times per minute. Here, the waiting time is set to 21 seconds to avoid triggering OpenAI's limit during batch translation. Improved compatibility of the plugin.

Implementation

When calling the onTranslateInBatch() function, it determines whether the current translation engine is GPT. If so, it uses the waiting time defined by batchTaskDelayGPT, otherwise it continues to use batchTaskDelay.

Issues

Batch translation can directly cause a slow translation feeling, but it does not affect non-GPT engines.

[Maasea]

OpenAI's limitation is only for users who are not tied to a credit card, you shouldn't have to fix the delay time, a better solution would be to offer an option

[lyk7539511]

OpenAI's limitation is only for users who are not tied to a credit card, you shouldn't have to fix the delay time, a better solution would be to offer an option

Copy

[lyk7539511]

Not yet completed.

[lyk7539511]

@Maasea please review

[Maasea]

1. You should add the content option in the GPT configuration dialog, see this link.
2. Since OpenAI's limitation is 3 times a minute, non-batch translation may also trigger this limit. The current code only handles batch translation.
3. Adding a judgment statement for GPT separately at the task may increase coupling, so perhaps it would be better to handle it in gpt.ts.
4. Forcing the setting to 21s may not be a good choice. For example, if the user only needs to call the API twice in 1 minute, they will not exceed the 3 times limit, but they will have to wait an additional 20 seconds.
5. It seems that the way to deal with the delay is to send a request every 21 seconds after 3 requests. It doesn't appear to satisfy the requirement of only sending 3 requests per minute, for example, if the first 3 requests are less than 40 seconds, the limits would still be triggered. Since my account is already bound to a credit card, I cannot actually test this case.
6. Users may also initiate multiple batch translations at the same time, which could lead to API restrictions.

Handling this restriction may require considering several usage scenarios, perhaps giving the user a toast when the restriction is triggered is a simple solution

@windingwind What is your opinion on this issue.

[lyk7539511]

I think your suggestions are all very excellent and I will reconsider my solution. @Maasea

[lyk7539511]

1. You should add the content option in the GPT configuration dialog, see this link.
2. Since OpenAI's limitation is 3 times a minute, non-batch translation may also trigger this limit. The current code only handles batch translation.
3. Adding a judgment statement for GPT separately at the task may increase coupling, so perhaps it would be better to handle it in gpt.ts.
4. ~Forcing the setting to 21s may not be a good choice. For example, if the user only needs to call the API twice in 1 minute, they will not exceed the 3 times limit, but they will have to wait an additional 20 seconds.~
5. It seems that the way to deal with the delay is to send a request every 21 seconds after 3 requests. It doesn't appear to satisfy the requirement of only sending 3 requests per minute, for example, if the first 3 requests are less than 40 seconds, the limits would still be triggered. Since my account is already bound to a credit card, I cannot actually test this case.
6. Users may also initiate multiple batch translations at the same time, which could lead to API restrictions.

Handling this restriction may require considering several usage scenarios, perhaps giving the user a toast when the restriction is triggered is a simple solution

@windingwind What is your opinion on this issue.

Sorry, I overestimated my ability. I've come up with a few solutions, but without exception, they all increase code coupling.

Your question is so good that it is too difficult for me to come up with a reasonable solution.

@Maasea

[Maasea]

I checked the official documentation, see here.

In fact, besides the number of times, there are also limitations on the Token.

In the description of the documentation, requests that trigger the limit seem to return specific messages. If it does, you should determine if the limit was triggered based on the message returned by the response, and then process it.

As mentioned earlier, I cannot trigger the limit in a short period of time, so I am unable to check the error response.

My thought is: just give a toast when the limit is generated, and there is little point in the translator doing the relevant checks locally, since you can't guarantee that there is only one API caller at the same time.

[lyk7539511]

Reconfirming again, we will allow translation tasks to time out and only provide a notification to the user.

[lyk7539511]

Hey guy, I have a problem that I can't solve. @Maasea

As shown in the code below, I have added a try-catch block to the code during runtime, and it will pop up a prompt box in the bottom right corner when an error occurs.

However, this code sometimes works and sometimes doesn't. I have tried my best to handle the errors, and this is the simplest way for me without considering extra error messages. Even in this case, the code will still run in a way that I cannot predict.

Do you have any good suggestions? (I have reverted all the previously modified files and only made changes to gpt.ts.)

gpt.ts

import { TranslateTaskProcessor } from "../../utils/translate";
import { getPref } from "../../utils/prefs";
import { getServiceSecret } from "../../utils/translate";

export const gptTranslate = <TranslateTaskProcessor>async function (data) {
  const model = getPref("gptModel");
  const temperature = parseFloat(getPref("gptTemperature") as string);
  const apiUrl = getPref("gptUrl");
  const xhr = await Zotero.HTTP.request(
    "POST",
    apiUrl,
    {
      headers: {
        "Content-Type": "application/json",
        Authorization: `Bearer ${data.secret}`,
      },
      body: JSON.stringify({
        model: model,
        messages: [
          {
            role: "user",
            content: `You are an academic expert with specialized knowledge in various fields. Please translate the following text from ${data.langfrom.split("-")[0]} into ${data.langto.split("-")[0]} using professional language  while ensuring the sentence flow:${data.raw}`,
          },
        ],
        temperature: temperature,
        stream: true,
      }),
      responseType: "text",
      requestObserver: (xmlhttp: XMLHttpRequest) => {
        let preLength = 0;
        let result = "";
        try {
          xmlhttp.onprogress = (e: any) => {
            // Only concatenate the new strings
            let newResponse = e.target.response.slice(preLength);
            let dataArray = newResponse.split("data: ");
            for (let data of dataArray) {
              try {
                let obj = JSON.parse(data);
                let choice = obj.choices[0];
                if (choice.finish_reason) {
                  break;
                }
                result += choice.delta.content || "";
              } catch {
                continue;
              }
            }
            // Clear timeouts caused by stream transfers
            if (e.target.timeout) {
              e.target.timeout = 0;
            }
            // Remove \n\n from the beginning of the data
            data.result = result.replace(/^\n\n/, "");
            preLength = e.target.response.length;
            if (data.type === "text") {
              addon.hooks.onReaderPopupRefresh();
              addon.hooks.onReaderTabPanelRefresh();
            }
          };
        } catch (error) {
            let text:string =  "You have triggered the call rate limit for OpenAI users who have not added payment methods, please be aware.";
            var progressWindow = new Zotero.ProgressWindow({ closeOnClick: true });
            progressWindow.changeHeadline("GPT error");
            progressWindow.addDescription(text);
            progressWindow.show();
            progressWindow.startCloseTimer(4000);
        }

      },
    }
  );
  if (xhr?.status !== 200) {
    throw `Request error: ${xhr?.status}`;
  }
  // data.result = xhr.response.choices[0].message.content.substr(2);
};

export const updateGPTModel = async function () {
  const secret = getServiceSecret("gpt");
  const xhr = await Zotero.HTTP.request(
    "GET",
    "https://api.openai.com/v1/models",
    {
      headers: {
        "Content-Type": "application/json",
        Authorization: `Bearer ${secret}`,
      },
      responseType: "json",
    }
  );

  const models = xhr.response.data;
  const availableModels = [];

  for (const model of models) {
    if (model.id.includes("gpt")) {
      availableModels.push(model.id);
    }
  }

  return availableModels;
};

[Maasea]

You can check the response body when the status is not equal to 200, and if the content contains restricted information, make a toast.

Although try catch can catch errors, it cannot determine whether they are caused by API limitations.

For exmaple：

if (xhr?.status !== 200) {
  /**
   *
   * Determine if the body content contains restricted information
   * If the api is restricted, make a toast
   *
   */

  throw `Request error: ${xhr?.status}`;
}

[lyk7539511]

You can check the response body when the status is not equal to 200, and if the content contains restricted information, make a toast.

Although try catch can catch errors, it cannot determine whether they are caused by API limitations.

For exmaple：

if (xhr?.status !== 200) {
  /**
   *
   * Determine if the body content contains restricted information
   * If the api is restricted, make a toast
   *
   */

  throw `Request error: ${xhr?.status}`;
}

I have tried your suggestion before, but it is not work.

[Maasea]

I rechecked the source code of Zotero's request section, and it will throw an exception when the status code is not 200.

You can try to judge the status in the onprogress function

requestObserver: (xmlhttp: XMLHttpRequest) => {
        let preLength = 0;
        let result = "";
        xmlhttp.onprogress = (e: any) => {

          if (e.target.status !== 200) {
           /**
             *
             * Determine if the body content contains restricted information
             * If the api is restricted, make a toast
             *
             */
          }

          // Only concatenate the new strings
          let newResponse = e.target.response.slice(preLength);
          let dataArray = newResponse.split("data: ");
          ......

[Maasea]

The original code have made a display for translation errors.

The issue seems to refer to no tips on the home page.

It would be a bit confusing if only the popup window indicated an exception for gpt translation errors and no indication from other translation engines.

I'm not sure if the @windingwind wants to show a popup on the home page when there is a translation error.

[lyk7539511]

This issue mainly focuses on triggering restrictions when translating paper titles in batch. The prompt on the PDF page of the paper is well done and does not need to be modified.

I only want to display error popup on the homepage. (I'm beginner in TS&Zotero and there are many things I need to learn.)

This error popup is for GPT and has nothing to do with other engines, so I cannot get your confusion.

So, @windingwind , what are your thoughts on this problem?

[windingwind]

Thank you all for your time and effort on this topic. I personally do not use GPT service here, so I may not represent GPT service users.

I only want to display error popup on the homepage.

In that case, you may use data.type === "title" in gpt.ts to detect if you are translating titles in the library and decide whether to show a popup (ztoolkit.ProgressWindow) or alert window (window.alert).

[Maasea]

This error popup is for GPT and has nothing to do with other engines, so I cannot get your confusion.

I am unsure if adding a toast straight to the run function is an optional fix because none of the current translation engines display a tip on the home page when an error occurs.

However, the code will not appear elegant in this way.

https://github.com/windingwind/zotero-pdf-translate/blob/fdd15848f177e65f9d7bbf4171960cfe3716a1c2/src/utils/translate.ts#L85

public async run(data: TranslateTask) {
    data.langfrom = getPref("sourceLanguage") as string;
    data.langto = getPref("targetLanguage") as string;
    data.secret = getServiceSecret(data.service);
    data.status = "processing";
    try {
      ztoolkit.log(data);
      await this.processor(data as Required<TranslateTask>);
      data.status = "success";
    } catch (e) {
      // only show on the homepage
      if (data.type == "abstract" || data.type == "title") {
        // make a toast
      }

      data.result = this.makeErrorInfo(data.service, String(e));
      data.status = "fail";
    }
  }

[lyk7539511]

This error popup is for GPT and has nothing to do with other engines, so I cannot get your confusion.

I am unsure if adding a toast straight to the run function is an optional fix because none of the current translation engines display a tip on the home page when an error occurs.

However, the code will not appear elegant in this way.

https://github.com/windingwind/zotero-pdf-translate/blob/fdd15848f177e65f9d7bbf4171960cfe3716a1c2/src/utils/translate.ts#L85

public async run(data: TranslateTask) {
    data.langfrom = getPref("sourceLanguage") as string;
    data.langto = getPref("targetLanguage") as string;
    data.secret = getServiceSecret(data.service);
    data.status = "processing";
    try {
      ztoolkit.log(data);
      await this.processor(data as Required<TranslateTask>);
      data.status = "success";
    } catch (e) {
      // only show on the homepage
      if (data.type == "abstract" || data.type == "title") {
        // make a toast
      }

      data.result = this.makeErrorInfo(data.service, String(e));
      data.status = "fail";
    }
  }

Your method works great and the code runs correctly. :) @Maasea

However, as you mentioned, it adds too much coupling and is not elegant enough.

So, I'd like to ask @windingwind if you could accept a solution that will increase code coupling. If you can accept it, I will push the new code and wait for merging. If you can't accept it, maybe it's time to close this PR.

My idea is to close this PR because the current code does not solve the problem we initially raised, but merely provides a simple notification.

[windingwind]

@lyk7539511 I'm OK with the modifications in the gpt.ts. As it only affects gpt, it is acceptable.

However, please do not touch the prefs.xhtml directly. This checkbox is only for gpt and thus should be added to the gpt configuration dialog.

Second this:

You should add the content option in the GPT configuration dialog, see this link.

[lyk7539511]

@windingwind Please close this PR. It's not feasible to achieve the desired function by only modifying gpt.ts.

Thank you for your cooperation and I hope we can have a better solution in the near future. Good luck with our collaboration. @windingwind @Maasea