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

bug: responses from /chat/completions endpoint contain a leading space in the content #2548

Closed
Propheticus opened this issue Mar 31, 2024 · 6 comments
Assignees
Labels
P2: nice to have Nice to have feature type: bug Something isn't working
Milestone

Comments

@Propheticus
Copy link

Propheticus commented Mar 31, 2024

Jan's API server responds with a leading space. This leads to broken output (markdown tables don't render right) and illegal file names when the output is used to generate note titles which are in turn used as the .md filename.

Call:

POST http://127.0.0.1:1337/v1/chat/completions
**headers**
content-type: application/json
**body**
{
  "messages": [
    {
      "content": "You are a helpful assistant.",
      "role": "system"
    },
    {
      "content": "Hello!",
      "role": "user"
    }
  ],
  "model": "mistral-ins-7b-q5",
  "stream": true,
  "max_tokens": 4096,
  "stop": [
    "</s>"
  ],
  "frequency_penalty": 0,
  "presence_penalty": 0,
  "temperature": 0.7,
  "top_p": 0.95
}

Response:

data: {"choices":[{"delta":{"content":" Hello"},"finish_reason":null,"index":0}],"created":1711886550,"id":"K75WwlMq7nBjqPW4FGlR","model":"_","object":"chat.completion.chunk"}
data: {"choices":[{"delta":{"content":" there"},"finish_reason":null,"index":0}],"created":1711886550,"id":"ADqnjtzqwx1zbUiWVXVm","model":"_","object":"chat.completion.chunk"}
...etc

Expected output:

data: {"choices":[{"delta":{"content":"Hello"},"finish_reason":null,"index":0}],"created":1711886550,"id":"K75WwlMq7nBjqPW4FGlR","model":"_","object":"chat.completion.chunk"}
data: {"choices":[{"delta":{"content":" there"},"finish_reason":null,"index":0}],"created":1711886550,"id":"ADqnjtzqwx1zbUiWVXVm","model":"_","object":"chat.completion.chunk"}
...etc

Tested with "stream": false as well and the same is true for un-chunked chat.completion objects.

@Propheticus
Copy link
Author

@Propheticus Propheticus changed the title bug: responses from /chat/completions endpoint contain a leading space in the first chunk bug: responses from /chat/completions endpoint contain a leading space in the content Mar 31, 2024
@Propheticus
Copy link
Author

Propheticus commented Mar 31, 2024

ggerganov/llama.cpp#3664 might be related? (would mean it's in nitro.exe which uses llama.cpp)
also: ggerganov/llama.cpp#367 (comment)

@Van-QA Van-QA added this to the v0.4.11 milestone Apr 1, 2024
@Van-QA Van-QA added the P2: nice to have Nice to have feature label Apr 1, 2024
@Propheticus
Copy link
Author

Reading the 2 issues above plus ggerganov/llama.cpp#4081 the leading space appears to be added during tokenization on purpose and is even needed for some models to work correctly.
I'm still unsure how/if tokenization, of what I thought was done to the input to be processed by a model, relates to the generation of a response.
Is the same done/needed for the output? Perhaps because of threads of messages where previous replies become context/input for the next prompt?

@Propheticus
Copy link
Author

Without going further into the rabbit hole of how tokenization works internally and whether it applies to completion....
The OpenAI API spec (and Mistral AI API as well) gives examples of expected response, without a leading space.

@louis-jan louis-jan moved this to Icebox in Jan & Cortex Apr 2, 2024
@louis-jan louis-jan moved this from Icebox to Planned in Jan & Cortex Apr 5, 2024
@louis-jan louis-jan assigned CameronNg and unassigned louis-jan Apr 5, 2024
@tikikun tikikun assigned vansangpfiev and unassigned CameronNg Apr 5, 2024
@0xSage 0xSage moved this from Planned to In Progress in Jan & Cortex Apr 9, 2024
@vansangpfiev vansangpfiev moved this from In Progress to In Review in Jan & Cortex Apr 10, 2024
@Van-QA Van-QA modified the milestones: v0.4.11, v0.4.12 Apr 10, 2024
@louis-jan louis-jan moved this from In Review to QA in Jan & Cortex Apr 16, 2024
@Van-QA
Copy link
Contributor

Van-QA commented Apr 17, 2024

hi @Propheticus, dev team resolved the issue, would you mind retrying it? many thanks 🙏

@Propheticus
Copy link
Author

Propheticus commented Apr 17, 2024

Looks good to me @Van-QA 👍
tested on Jan v0.4.11-386 nightly.

@Van-QA Van-QA closed this as completed Apr 17, 2024
@github-project-automation github-project-automation bot moved this from QA to Done in Jan & Cortex Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2: nice to have Nice to have feature type: bug Something isn't working
Projects
Archived in project
Development

No branches or pull requests

5 participants