Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions py/chat.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def initialize_chat_window():
vim.command("redraw")

file_content = vim.eval('trim(join(getline(1, "$"), "\n"))')
role_lines = re.findall(r'(^>>> user|^>>> system|^<<< assistant).*', file_content, flags=re.MULTILINE)
role_lines = re.findall(r'(^>>> user|^>>> system|^<<< thinking|^<<< assistant).*', file_content, flags=re.MULTILINE)
if not role_lines[-1].startswith(">>> user"):
# last role is not user, most likely completion was cancelled before
vim.command("normal! o")
Expand Down Expand Up @@ -58,14 +58,26 @@ def initialize_chat_window():
try:
last_content = messages[-1]["content"][-1]
if last_content['type'] != 'text' or last_content['text']:
vim.command("normal! Go\n<<< assistant\n\n")
vim.command("redraw")

print('Answering...')
vim.command("redraw")

first_thinking_chunk = True
first_content_chunk = True

text_chunks = make_chat_text_chunks(messages, options)
render_text_chunks(text_chunks)
for chunk in text_chunks:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here it would be nice to re-use render_text_chunks helper function, it covers more functionality like AIRedo. For example filter thinking chunks first, then if not empty render it with render_text_chunks and then handle content

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me it's confusing how render_text_chunks handles insertion to support in-place edits vs. chat.
If you want this in the render_text_chunks function, could you please first check if you agree with this change #148 ?

if text := chunk.get("thinking"):
if first_thinking_chunk:
first_thinking_chunk = False
vim.command("normal! Go\n<<< thinking\n\n")
vim.command("normal! A" + text)
if text := chunk.get("content"):
if first_content_chunk:
first_content_chunk = False
vim.command("normal! Go\n<<< assistant\n\n")
vim.command("normal! A" + text)
vim.command("redraw")

vim.command("normal! a\n\n>>> user\n\n")
vim.command("redraw")
Expand Down
5 changes: 4 additions & 1 deletion py/complete.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ def chat_engine(prompt):
chat_content = f"{initial_prompt}\n\n>>> user\n\n{prompt}".strip()
messages = parse_chat_messages(chat_content)
print_debug("[engine-chat] text:\n" + chat_content)
return make_chat_text_chunks(messages, config_options)
return filter(
lambda c: c,
map(lambda c: c.get("content"), make_chat_text_chunks(messages, config_options)),
)

engines = {"chat": chat_engine, "complete": complete_engine}

Expand Down
26 changes: 21 additions & 5 deletions py/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,12 @@ def parse_chat_messages(chat_content):
for line in lines:
match line:
case '>>> system':
messages.append({'role': 'system', 'content': [{ 'type': 'text', 'text': '' }]})
messages.append({'role': 'system', 'content': ''})
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this change needed? that also breaks unit tests.
Btw. it would be nice to have an unit test checking thinking section is omitted (tests/chat_test.py)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a regression introduced by some recent change. Perhaps should have been a separate PR.

A content array is not supported by some providers, namely:

  1. groq:
{"error":{"message":"'messages.0' : for 'role:system' the following must be satisfied[('messages.0.content' : value must be a string)]","type":"invalid_request_error"}}
  1. deepseek:
Failed to deserialize the JSON body into the target type: messages[0]: data did not match any variant of untagged enum ChatCompletionRequestContent at line 1 column 109

I haven't looked at the tests yet because I wanted to know if you will accept this PR.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, sounds good

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I may ask you to keep the previous structured format here and move this logic to make_chat_text_chunks - reformat text messages before the request. that would makes things easier when I'll be merging custom providers support.

current_type = 'system'
case '<<< thinking':
current_type = 'thinking'
case '<<< assistant':
messages.append({'role': 'assistant', 'content': [{ 'type': 'text', 'text': '' }]})
messages.append({'role': 'assistant', 'content': ''})
current_type = 'assistant'
case '>>> user':
if messages and messages[-1]['role'] == 'user':
Expand All @@ -175,7 +177,9 @@ def parse_chat_messages(chat_content):
if not messages:
continue
match current_type:
case 'assistant' | 'system' | 'user':
case 'system' | 'assistant':
messages[-1]['content'] += '\n' + line
case 'user':
messages[-1]['content'][-1]['text'] += '\n' + line
case 'include':
paths = parse_include_paths(line)
Expand All @@ -185,6 +189,10 @@ def parse_chat_messages(chat_content):

for message in messages:
# strip newlines from the text content as it causes empty responses

if isinstance(message['content'], str):
message['content'] = message['content'].strip()
continue
for content in message['content']:
if content['type'] == 'text':
content['text'] = content['text'].strip()
Expand Down Expand Up @@ -334,11 +342,19 @@ def _choices(resp):

def map_chunk_no_stream(resp):
print_debug("[engine-chat] response: {}", resp)
return _choices(resp)[0].get('message', {}).get('content', '')
message = _choices(resp)[0].get('message', {})
reasoning_content = message.get('reasoning_content', '')
content = message.get('content', '')
return {"content": content, "thinking": reasoning_content}

def map_chunk_stream(resp):
print_debug("[engine-chat] response: {}", resp)
return _choices(resp)[0].get('delta', {}).get('content', '')
delta = _choices(resp)[0].get('delta', {})
if reasoning_content := delta.get('reasoning_content'):
return {"thinking": reasoning_content}
if content := delta.get('content'):
return {"content": content}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is no special reason, handling the chunk the same way as in map_chunk_no_stream would make it probably simpler, e.g.:

delta = _choices(resp)[0].get('message', {})
reasoning_content = delta.get('reasoning_content', '')
content = delta.get('content', '')
return {"content": content, "thinking": reasoning_content}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason is that a chunk can have {"content": None}, so delta.get('content', '') would return None, not an empty string. But I can simplify it taking inspiration from #150

return {"content": ""}

map_chunk = map_chunk_stream if openai_options['stream'] else map_chunk_no_stream

Expand Down
1 change: 1 addition & 0 deletions syntax/aichat.vim
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
syntax match aichatRole ">>> system"
syntax match aichatRole ">>> user"
syntax match aichatRole ">>> include"
syntax match aichatRole "<<< thinking"
syntax match aichatRole "<<< assistant"

highlight default link aichatRole Comment