Skip to content
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

Fix maxContextLength not being passed to openai adapter #94

Closed
wants to merge 1 commit into from

Conversation

khanonnie
Copy link
Contributor

@khanonnie khanonnie commented Mar 27, 2023

This PR partially addresses #93 but may not resolve the problem for other adapters. A preset's maxContextLength was being incorrectly stripped from the settings object used by the openai adapter to figure out its token budget, causing it to fall back to the basic preset value of 2048. Might be happening for other adapters too, but maybe obfuscated by most other services not supporting >2048 context to begin with.

Better solution might be to special-case this value rather than add it to every serviceGenMap since it seems like it's the kind of thing that 1) every service requires and 2) doesn't even need to be sent to the downstream service at all since it's used to build the prompt within Agnai.

Edit: Following scueick's clarification I think this probably doesn't happen on any other adapters and is specific to Turbo because of how it has to transform the prompt built by the client into the weird format expected by OAI's chat endpoint.

@khanonnie khanonnie changed the title Add maxContextLength to openai's serviceGenMap Fix maxContextLength not being passed to openai adapter Mar 27, 2023
@sceuick
Copy link
Member

sceuick commented Mar 27, 2023

The token budget typically isn't used by the adapters. It's mainly used when building the prompt so this shouldn't be necessary.
If the final prompt isn't being filled correctly, it may be a client-side issue as that's where the "parts" as built before being sent to the API.
It's possible that the maxContextLength that the client-side is using isn't the correct value.

@khanonnie
Copy link
Contributor Author

khanonnie commented Mar 27, 2023

The token budget typically isn't used by the adapters. It's mainly used when building the prompt so this shouldn't be necessary. If the final prompt isn't being filled correctly, it may be a client-side issue as that's where the "parts" as built before being sent to the API. It's possible that the maxContextLength that the client-side is using isn't the correct value.

@sceuick I think this may be specific to turbo, I didn't realize prompt building normally happens on the client. Turbo has a special case that does some of it on the adapter side on the server; I was poking around in the debugger and this specifically was the problematic code:

// srv/adapter/openai.ts#handleOAI
    const all = []

    let maxBudget =
      (settings.maxContextLength || defaultPresets.basic.maxContextLength) - settings.max_tokens

    let tokens = encoder(parts.gaslight)

    if (lines) {
      all.push(...lines)
    }

I was seeing all of the parts being sent by the client correctly (since it had the right value for maxContextSize) but the OAI adapter on the server also appears to need this value, I guess in order to transform the parts into Turbo's unique chatlog/message array payload format. In the debugger, settings.maxContextLength is undefined so it falls back to 2048 for the max context size and ends up stripping a huge part of the parts sent by the client.

So maybe adding maxContextLength to OAI's serviceGenMap is enough to fix this for just Turbo, and nothing needs to change for any other adapters since as you say the parts are built on the client.

@sceuick
Copy link
Member

sceuick commented Mar 27, 2023

Aaah I see! The settings from the preset should be passed through. I think I need to be using gen.maxContextLength instead of settings.maxContextLength

@sceuick
Copy link
Member

sceuick commented Mar 27, 2023

I've fixed this in the PR I just opened.

@sceuick
Copy link
Member

sceuick commented Mar 27, 2023

This should be fixed in the main branch now.

@sceuick sceuick closed this Mar 27, 2023
@khanonnie khanonnie deleted the fix-openai-maxcontext branch September 27, 2023 04:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants