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

Replacement parser inverse #172

Merged
merged 4 commits into from
Oct 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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
10 changes: 3 additions & 7 deletions mentat/parsers/block_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,15 +199,11 @@ def _add_code_block(
)
return ""

def file_edits_to_llm_message(
self, parsedLLMResponse: ParsedLLMResponse, git_root: Path | None = None
) -> str:
def file_edits_to_llm_message(self, parsedLLMResponse: ParsedLLMResponse) -> str:
"""
Inverse of stream_and_parse_llm_response;
takes in the generated message and file edits and returns the original message
Inverse of stream_and_parse_llm_response
"""
if git_root is None:
git_root = GIT_ROOT.get()
git_root = GIT_ROOT.get()
ans = parsedLLMResponse.conversation
for file_edit in parsedLLMResponse.file_edits:
tmp = {}
Expand Down
42 changes: 41 additions & 1 deletion mentat/parsers/replacement_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@

from mentat.code_file_manager import CodeFileManager
from mentat.errors import ModelError
from mentat.git_handler import GIT_ROOT
from mentat.parsers.change_display_helper import DisplayInformation, FileActionType
from mentat.parsers.file_edit import FileEdit, Replacement
from mentat.parsers.parser import Parser
from mentat.parsers.parser import ParsedLLMResponse, Parser
from mentat.prompts.prompts import read_prompt

replacement_parser_prompt_filename = Path("replacement_parser_prompt.txt")
Expand Down Expand Up @@ -119,3 +120,42 @@ def _add_code_block(
)
)
return ""

def file_edits_to_llm_message(self, parsedLLMResponse: ParsedLLMResponse) -> str:
"""
Inverse of stream_and_parse_llm_response
"""
git_root = GIT_ROOT.get()
ans = parsedLLMResponse.conversation
for file_edit in parsedLLMResponse.file_edits:
action_indicator = ""
if file_edit.is_creation:
action_indicator = "+"
elif file_edit.is_deletion:
action_indicator = "-"
elif file_edit.rename_file_path is not None:
action_indicator = file_edit.rename_file_path.as_posix()
Copy link
Member

Choose a reason for hiding this comment

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

I could be wrong, but isn't the rename_file_path also absolute? I think we also need to get it relative to the git root here too

Copy link
Member Author

Choose a reason for hiding this comment

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

No actually. For both this parser and the block parser it ends up as a relative path. Maybe a bug in the parser if that's not what you intended.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, that is a bug in the parser that could actually have some problems; I'll have to fix that (in a separate pr)

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, that is a bug in the parser that could actually have some problems; I'll have to fix that (in a separate pr)

can you make an issue for this @PCSwingle unless you are going ahead and doing it now


file_rel_path = file_edit.file_path.relative_to(git_root).as_posix()
if action_indicator != "":
ans += f"@ {file_rel_path} {action_indicator}\n"

for replacement in file_edit.replacements:
if (
replacement.starting_line == replacement.ending_line
and len(replacement.new_lines) != 0
):
ans += (
f"@ {file_rel_path} insert_line={replacement.starting_line + 1}\n"
)
else:
ans += (
f"@ {file_rel_path} starting_line={replacement.starting_line + 1} "
f"ending_line={replacement.ending_line}\n"
)

if len(replacement.new_lines) != 0:
ans += "\n".join(replacement.new_lines) + "\n"
ans += "@\n"

return ans
85 changes: 2 additions & 83 deletions tests/parser_tests/block_format_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@

from mentat.parsers.block_parser import BlockParser
from mentat.session import Session
from mentat.utils import convert_string_to_asyncgen
from tests.conftest import ConfigManager
from tests.parser_tests.inverse import verify_inverse


@pytest.fixture(autouse=True)
Expand Down Expand Up @@ -458,88 +458,7 @@ async def test_inverse(
mock_code_file_manager,
mock_git_root,
):
# This test verifies that the inverse is a left inverse to stream_and_parse_llm_response. That is we can go:
# llm_message -> file_edits -> llm_message
# and get back where we started. Actually what we care about is that it's a right inverse. That if we go:
# file_edits -> llm_message -> file_edits
# we get back where we started. So this test verifies things we don't necessarily care about like the order of the
# edits and white space.
llm_response = dedent("""\
I will insert a comment between the first two lines
and then replace the last line with 'better measure'
Steps: 1. Insert a comment
2. Replace last line
@@start
{
"file": "test.txt",
"action": "insert",
"insert-after-line": 1,
"insert-before-line": 2
}
@@code
# I inserted this comment
@@end
@@start
{
"file": "test.txt",
"action": "replace",
"start-line": 4,
"end-line": 4
}
@@code
# better measure
@@end
@@start
{
"file": "delete.txt",
"action": "delete",
"start-line": 2,
"end-line": 3
}
@@end
@@start
{
"file": "create.txt",
"action": "create-file"
}
@@code
# I created this file
@@end
@@start
{
"file": "file1.txt",
"action": "rename-file",
"name": "file2.txt"
}
@@end
@@start
{
"file": "file1.txt",
"action": "insert",
"insert-after-line": 1,
"insert-before-line": 2
}
@@code
# I inserted this comment in a replacement
@@end
@@start
{
"file": "file1.txt",
"action": "replace",
"start-line": 4,
"end-line": 4
}
@@code
# better measure in a replacement
@@end
""")

generator = convert_string_to_asyncgen(llm_response, 10)
parser = BlockParser()
parsedLLMResponse = await parser.stream_and_parse_llm_response(generator)
inverse = parser.file_edits_to_llm_message(parsedLLMResponse)

assert llm_response == inverse
await verify_inverse(BlockParser())


@pytest.mark.asyncio
Expand Down
88 changes: 88 additions & 0 deletions tests/parser_tests/inverse.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import os
from pathlib import Path
from textwrap import dedent

from mentat.parsers.file_edit import FileEdit, Replacement
from mentat.parsers.parser import ParsedLLMResponse
from mentat.utils import convert_string_to_asyncgen


async def verify_inverse(parser):
Copy link
Member

Choose a reason for hiding this comment

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

This is really nice, thanks for making this!

cwd = Path(os.getcwd())
parsedLLMResponse = ParsedLLMResponse(
full_response="",
conversation=dedent("""\
Conversation
with two lines
"""),
file_edits=[
FileEdit(
file_path=Path(cwd / "test.txt"),
replacements=[
Replacement(
starting_line=1,
ending_line=1,
new_lines=["# I inserted this comment"],
),
Replacement(
starting_line=3, ending_line=4, new_lines=["# better measure"]
),
],
is_creation=False,
is_deletion=False,
rename_file_path=None,
),
FileEdit(
file_path=Path(cwd / "delete.txt"),
replacements=[
Replacement(starting_line=1, ending_line=3, new_lines=[])
],
is_creation=False,
is_deletion=False,
rename_file_path=None,
),
FileEdit(
file_path=Path(cwd / "create.txt"),
replacements=[
Replacement(
starting_line=0,
ending_line=0,
new_lines=["# I created this file"],
)
],
is_creation=True,
is_deletion=False,
rename_file_path=None,
),
FileEdit(
file_path=Path(cwd / "file1.txt"),
replacements=[
Replacement(
starting_line=1,
ending_line=1,
new_lines=["# I inserted this comment in a replacement"],
),
Replacement(
starting_line=3,
ending_line=4,
new_lines=["# better measure in a replacement"],
),
],
is_creation=False,
is_deletion=False,
rename_file_path=Path("file2.txt"),
),
],
)
inverse = parser.file_edits_to_llm_message(parsedLLMResponse)
generator = convert_string_to_asyncgen(inverse, 10)
back_once = await parser.stream_and_parse_llm_response(generator)
inverse2 = parser.file_edits_to_llm_message(back_once)
generator2 = convert_string_to_asyncgen(inverse2, 10)
back_twice = await parser.stream_and_parse_llm_response(generator2)

# The full_response is originally blank for compatibility with all formats. So we invert twice.
assert parsedLLMResponse.file_edits == back_once.file_edits
assert back_once == back_twice
# Verify the inverse uses relative paths
assert "testbed" not in back_once.full_response
14 changes: 14 additions & 0 deletions tests/parser_tests/replacement_format_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@

import pytest

from mentat.parsers.replacement_parser import ReplacementParser
from mentat.session import Session
from tests.conftest import ConfigManager
from tests.parser_tests.inverse import verify_inverse


@pytest.fixture(autouse=True)
Expand Down Expand Up @@ -246,3 +248,15 @@ async def test_change_then_rename_then_change(
# New line 2
# with 2 lines""")
assert content == expected_content


@pytest.mark.asyncio
async def test_inverse(
mock_stream,
mock_call_llm_api,
mock_collect_user_input,
mock_setup_api_key,
mock_code_file_manager,
mock_git_root,
):
await verify_inverse(ReplacementParser())