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.

Sign up for GitHub

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

Refactor Xassist, Test Framework #161

Merged
merged 1 commit into from
Oct 18, 2024
Merged

Refactor Xassist, Test Framework #161

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
61 changes: 55 additions & 6 deletions src/xmagics/xassist.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
using json = nlohmann::json;

// TODO: Implement xplugin to separate the magics from the main code.
// TODO: Add support for open-source models.
namespace xcpp
{
class api_key_manager
Expand Down Expand Up @@ -279,6 +278,53 @@ namespace xcpp
}
};

std::string escape_special_cases(const std::string& input)
{
std::string escaped;
for (char c : input)
{
switch (c)
{
case '\\':
escaped += "\\\\";
break;
case '\"':
escaped += "\\\"";
break;
case '\n':
escaped += "\\n";
break;
case '\t':
escaped += "\\t";
break;
case '\r':
escaped += "\\r";
break;
case '\b':
escaped += "\\b";
break;
case '\f':
escaped += "\\f";
break;
default:
if (c < 0x20 || c > 0x7E)
{
// Escape non-printable ASCII characters and non-ASCII characters
std::array<char, 7> buffer{};
std::stringstream ss;
ss << "\\u" << std::hex << std::setw(4) << std::setfill('0') << (c & 0xFFFF);
escaped += ss.str();
}
else
{
escaped += c;
}
break;
}
}
return escaped;
}

Copy link
Collaborator

@anutosh491 anutosh491 Oct 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This PR takes care of scenarios when the prompt has special characters or anything involving disrupting the JSON to call the LLM.

Hmm, so should there be a test required in this PR to check this claim ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, so should there be a test required in this PR to check this claim ?

I did try but got in a predicament where escape_special_cases("""{}") == "\"\"{}" yields false but works in the kernel. So I tested them manually and pushed the code

std::string gemini(const std::string& cell, const std::string& key)
{
curl_helper curl_helper;
Expand Down Expand Up @@ -369,8 +415,8 @@ namespace xcpp
}

const std::string post_data = R"({
"model": [)" + model
+ R"(],
"model": ")" + model
+ R"(",
"messages": [)" + chat_message
+ R"(],
"temperature": 0.7
Expand Down Expand Up @@ -453,18 +499,21 @@ namespace xcpp
}
}


const std::string prompt = escape_special_cases(cell);

std::string response;
if (model == "gemini")
{
response = gemini(cell, key);
response = gemini(prompt, key);
}
else if (model == "openai")
{
response = openai(cell, key);
response = openai(prompt, key);
}
else if (model == "ollama")
{
response = ollama(cell);
response = ollama(prompt);
}

std::cout << response;
Expand Down
2 changes: 0 additions & 2 deletions test/test_xcpp_kernel.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,6 @@ def test_notebooks(self):
with open(out) as f:
output_nb = nbformat.read(f, as_version=4)

check = True

# Iterate over the cells in the input and output notebooks
for i, (input_cell, output_cell) in enumerate(zip(input_nb.cells, output_nb.cells)):
if input_cell.cell_type == 'code' and output_cell.cell_type == 'code':
Expand Down