From 76257cd46a7bccb28e9b359a83c9bf32eadd2cf7 Mon Sep 17 00:00:00 2001 From: Roger Zurawicki Date: Thu, 2 Mar 2023 18:12:30 -0500 Subject: [PATCH 1/3] Add tests and benchmarks for githook and config operations - Add tests for version, configuration, installation, githook, and benchmark commands - Test the git hook file ignore feature - Change the installation command to include `--offline` - Change default OpenAI model to `text-davinci-003` - Add checks for output of `gptcommit config set --local` and `gptcommit config delete --local` commands --- Justfile | 5 ++++- tests/bench/bench_githook_openai.sh | 6 ++++++ tests/bench/bench_githook_test.sh | 7 +++++++ tests/bench/run_bench.sh | 21 +++++++++++++++++++ .../e2e}/test_config_list_get_set.sh | 0 .../e2e}/test_config_local_list_get_set.sh | 0 {e2e => tests/e2e}/test_githook.sh | 0 .../e2e}/test_githook_file_ignore.sh | 0 {e2e => tests/e2e}/test_install.sh | 0 {e2e => tests/e2e}/test_version.sh | 0 10 files changed, 38 insertions(+), 1 deletion(-) create mode 100755 tests/bench/bench_githook_openai.sh create mode 100755 tests/bench/bench_githook_test.sh create mode 100755 tests/bench/run_bench.sh rename {e2e => tests/e2e}/test_config_list_get_set.sh (100%) rename {e2e => tests/e2e}/test_config_local_list_get_set.sh (100%) rename {e2e => tests/e2e}/test_githook.sh (100%) rename {e2e => tests/e2e}/test_githook_file_ignore.sh (100%) rename {e2e => tests/e2e}/test_install.sh (100%) rename {e2e => tests/e2e}/test_version.sh (100%) diff --git a/Justfile b/Justfile index d55b997..5b03e89 100644 --- a/Justfile +++ b/Justfile @@ -22,12 +22,15 @@ install: cargo install --path . e2e: install - sh -eux -c 'for i in ./e2e/test_*.sh ; do sh -x "$i" ; done' + sh -eux -c 'for i in ./tests/e2e/test_*.sh ; do sh -x "$i" ; done' test *args: e2e cargo test alias t := test +bench: install + sh ./tests/bench/run_bench.sh + lint: cargo fmt --all -- --check cargo clippy --all-features --all-targets -- -D warnings --allow deprecated diff --git a/tests/bench/bench_githook_openai.sh b/tests/bench/bench_githook_openai.sh new file mode 100755 index 0000000..2e58859 --- /dev/null +++ b/tests/bench/bench_githook_openai.sh @@ -0,0 +1,6 @@ +#!/bin/sh + +gptcommit prepare-commit-msg \ + --git-diff-content "${1}" \ + --commit-msg-file "${2}" \ + --commit-source "" diff --git a/tests/bench/bench_githook_test.sh b/tests/bench/bench_githook_test.sh new file mode 100755 index 0000000..3f5a5c2 --- /dev/null +++ b/tests/bench/bench_githook_test.sh @@ -0,0 +1,7 @@ +#!/bin/sh + +GPTCOMMIT__MODEL_PROVIDER="tester-foobar" \ +gptcommit prepare-commit-msg \ + --git-diff-content "${1}" \ + --commit-msg-file "${2}" \ + --commit-source "" diff --git a/tests/bench/run_bench.sh b/tests/bench/run_bench.sh new file mode 100755 index 0000000..a0959d7 --- /dev/null +++ b/tests/bench/run_bench.sh @@ -0,0 +1,21 @@ +#!/bin/sh +set -eu + +ROOT="$(pwd)" +DIFF_CONTENT_PATH="$(pwd)/tests/data/example_1.diff" + +export TEMPDIR=$(mktemp -d) +( + cd "${TEMPDIR}" + git init + + export TEMPFILE=$(mktemp) + echo "foo" > $TEMPFILE + + hyperfine \ + --warmup 1 \ + --max-runs 10 \ + "$ROOT/tests/bench/bench_githook_test.sh"\ "${DIFF_CONTENT_PATH}"\ "${TEMPFILE}" \ + "$ROOT/tests/bench/bench_githook_openai.sh"\ "${DIFF_CONTENT_PATH}"\ "${TEMPFILE}" \ +) +rm -rf "${TEMPDIR}" diff --git a/e2e/test_config_list_get_set.sh b/tests/e2e/test_config_list_get_set.sh similarity index 100% rename from e2e/test_config_list_get_set.sh rename to tests/e2e/test_config_list_get_set.sh diff --git a/e2e/test_config_local_list_get_set.sh b/tests/e2e/test_config_local_list_get_set.sh similarity index 100% rename from e2e/test_config_local_list_get_set.sh rename to tests/e2e/test_config_local_list_get_set.sh diff --git a/e2e/test_githook.sh b/tests/e2e/test_githook.sh similarity index 100% rename from e2e/test_githook.sh rename to tests/e2e/test_githook.sh diff --git a/e2e/test_githook_file_ignore.sh b/tests/e2e/test_githook_file_ignore.sh similarity index 100% rename from e2e/test_githook_file_ignore.sh rename to tests/e2e/test_githook_file_ignore.sh diff --git a/e2e/test_install.sh b/tests/e2e/test_install.sh similarity index 100% rename from e2e/test_install.sh rename to tests/e2e/test_install.sh diff --git a/e2e/test_version.sh b/tests/e2e/test_version.sh similarity index 100% rename from e2e/test_version.sh rename to tests/e2e/test_version.sh From b217194541cbe23d786a306eaecf2ff8c1e4919c Mon Sep 17 00:00:00 2001 From: Roger Zurawicki Date: Thu, 2 Mar 2023 18:32:42 -0500 Subject: [PATCH 2/3] Add async_openai API for completions --- Cargo.lock | 279 +++++++++++++++++++++++++++++++++++++++++++++ Cargo.toml | 1 + src/llms/openai.rs | 85 +++++--------- 3 files changed, 306 insertions(+), 59 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index adeebe3..070857d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -133,6 +133,28 @@ dependencies = [ "futures-lite", ] +[[package]] +name = "async-openai" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c791cd9568241317f49bb3e3a6b595c986a2022428caa16a316be2520d05acf1" +dependencies = [ + "backoff", + "base64 0.21.0", + "derive_builder", + "futures", + "rand", + "reqwest", + "reqwest-eventsource", + "serde", + "serde_json", + "thiserror", + "tokio", + "tokio-stream", + "tokio-util", + "tracing", +] + [[package]] name = "async-std" version = "1.12.0" @@ -199,6 +221,20 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d468802bab17cbc0cc575e9b053f41e72aa36bfa6b7f55e3529ffa43161b97fa" +[[package]] +name = "backoff" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b62ddb9cb1ec0a098ad4bbf9344d0713fa193ae1a80af55febcff2627b6a00c1" +dependencies = [ + "futures-core", + "getrandom", + "instant", + "pin-project-lite", + "rand", + "tokio", +] + [[package]] name = "base64" version = "0.13.1" @@ -451,6 +487,72 @@ dependencies = [ "syn", ] +[[package]] +name = "darling" +version = "0.14.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c0808e1bd8671fb44a113a14e13497557533369847788fa2ae912b6ebfce9fa8" +dependencies = [ + "darling_core", + "darling_macro", +] + +[[package]] +name = "darling_core" +version = "0.14.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "001d80444f28e193f30c2f293455da62dcf9a6b29918a4253152ae2b1de592cb" +dependencies = [ + "fnv", + "ident_case", + "proc-macro2", + "quote", + "strsim", + "syn", +] + +[[package]] +name = "darling_macro" +version = "0.14.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b36230598a2d5de7ec1c6f51f72d8a99a9208daff41de2084d06e3fd3ea56685" +dependencies = [ + "darling_core", + "quote", + "syn", +] + +[[package]] +name = "derive_builder" +version = "0.12.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8d67778784b508018359cbc8696edb3db78160bab2c2a28ba7f56ef6932997f8" +dependencies = [ + "derive_builder_macro", +] + +[[package]] +name = "derive_builder_core" +version = "0.12.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c11bdc11a0c47bc7d37d582b5285da6849c96681023680b906673c5707af7b0f" +dependencies = [ + "darling", + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "derive_builder_macro" +version = "0.12.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ebcda35c7a396850a55ffeac740804b40ffec779b98fffbb1738f4033f0ee79e" +dependencies = [ + "derive_builder_core", + "syn", +] + [[package]] name = "digest" version = "0.10.6" @@ -529,6 +631,17 @@ version = "2.5.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0206175f82b8d6bf6652ff7d71a1e27fd2e4efde587fd368662814d6ec1d9ce0" +[[package]] +name = "eventsource-stream" +version = "0.2.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "74fef4569247a5f429d9156b9d0a2599914385dd189c539334c625d8099d90ab" +dependencies = [ + "futures-core", + "nom", + "pin-project-lite", +] + [[package]] name = "fancy-regex" version = "0.11.0" @@ -588,6 +701,21 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "futures" +version = "0.3.26" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "13e2792b0ff0340399d58445b88fd9770e3489eff258a4cbc1523418f12abf84" +dependencies = [ + "futures-channel", + "futures-core", + "futures-executor", + "futures-io", + "futures-sink", + "futures-task", + "futures-util", +] + [[package]] name = "futures-channel" version = "0.3.26" @@ -595,6 +723,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2e5317663a9089767a1ec00a487df42e0ca174b61b4483213ac24448e4664df5" dependencies = [ "futures-core", + "futures-sink", ] [[package]] @@ -603,6 +732,17 @@ version = "0.3.26" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ec90ff4d0fe1f57d600049061dc6bb68ed03c7d2fbd697274c41805dcb3f8608" +[[package]] +name = "futures-executor" +version = "0.3.26" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e8de0a35a6ab97ec8869e32a2473f4b1324459e14c29275d14b10cb1fd19b50e" +dependencies = [ + "futures-core", + "futures-task", + "futures-util", +] + [[package]] name = "futures-io" version = "0.3.26" @@ -624,6 +764,17 @@ dependencies = [ "waker-fn", ] +[[package]] +name = "futures-macro" +version = "0.3.26" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "95a73af87da33b5acf53acfebdc339fe592ecf5357ac7c0a7734ab9d8c876a70" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "futures-sink" version = "0.3.26" @@ -636,16 +787,28 @@ version = "0.3.26" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dcf79a1bf610b10f42aea489289c5a2c478a786509693b80cd39c44ccd936366" +[[package]] +name = "futures-timer" +version = "3.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e64b03909df88034c26dc1547e8970b91f98bdb65165d6a4e9110d94263dbb2c" + [[package]] name = "futures-util" version = "0.3.26" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9c1d6de3acfef38d2be4b1f543f553131788603495be83da675e180c8d6b7bd1" dependencies = [ + "futures-channel", "futures-core", + "futures-io", + "futures-macro", + "futures-sink", "futures-task", + "memchr", "pin-project-lite", "pin-utils", + "slab", ] [[package]] @@ -710,6 +873,7 @@ name = "gptcommit" version = "0.2.2" dependencies = [ "anyhow", + "async-openai", "async-std", "async-trait", "clap", @@ -876,6 +1040,12 @@ dependencies = [ "tokio-native-tls", ] +[[package]] +name = "ident_case" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b9e0384b61958566e926dc50660321d12159025e767c18e043daf26b70104c39" + [[package]] name = "idna" version = "0.3.0" @@ -1041,6 +1211,16 @@ version = "0.3.16" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2a60c7ce501c71e03a9c9c0d35b861413ae925bd979cc7a4e30d060069aaac8d" +[[package]] +name = "mime_guess" +version = "2.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4192263c238a5f0d0c6bfd21f336a313a4ce1c450542449ca191bb657b4642ef" +dependencies = [ + "mime", + "unicase", +] + [[package]] name = "minimal-lexical" version = "0.2.1" @@ -1299,6 +1479,12 @@ dependencies = [ "windows-sys 0.42.0", ] +[[package]] +name = "ppv-lite86" +version = "0.2.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5b40af805b3121feab8a3c29f04d8ad262fa8e0561883e7653e024ae4479e6de" + [[package]] name = "proc-macro-error" version = "1.0.4" @@ -1341,6 +1527,36 @@ dependencies = [ "proc-macro2", ] +[[package]] +name = "rand" +version = "0.8.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "34af8d1a0e25924bc5b7c43c079c942339d8f0a8b57c39049bef581b46327404" +dependencies = [ + "libc", + "rand_chacha", + "rand_core", +] + +[[package]] +name = "rand_chacha" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e6c10a63a0fa32252be49d21e7709d4d4baf8d231c2dbce1eaa8141b9b127d88" +dependencies = [ + "ppv-lite86", + "rand_core", +] + +[[package]] +name = "rand_core" +version = "0.6.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ec0be4795e2f6a28069bec0b5ff3e2ac9bafc99e6a9a7dc3547996c5c816922c" +dependencies = [ + "getrandom", +] + [[package]] name = "redox_syscall" version = "0.2.16" @@ -1406,6 +1622,7 @@ dependencies = [ "js-sys", "log", "mime", + "mime_guess", "native-tls", "once_cell", "percent-encoding", @@ -1423,11 +1640,28 @@ dependencies = [ "url", "wasm-bindgen", "wasm-bindgen-futures", + "wasm-streams", "web-sys", "webpki-roots", "winreg", ] +[[package]] +name = "reqwest-eventsource" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8f03f570355882dd8d15acc3a313841e6e90eddbc76a93c748fd82cc13ba9f51" +dependencies = [ + "eventsource-stream", + "futures-core", + "futures-timer", + "mime", + "nom", + "pin-project-lite", + "reqwest", + "thiserror", +] + [[package]] name = "ring" version = "0.16.20" @@ -1905,6 +2139,17 @@ dependencies = [ "webpki", ] +[[package]] +name = "tokio-stream" +version = "0.1.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8fb52b74f05dbf495a8fba459fdc331812b96aa086d9eb78101fa0d4569c3313" +dependencies = [ + "futures-core", + "pin-project-lite", + "tokio", +] + [[package]] name = "tokio-util" version = "0.7.7" @@ -1976,9 +2221,21 @@ checksum = "8ce8c33a8d48bd45d624a6e523445fd21ec13d3653cd51f681abf67418f54eb8" dependencies = [ "cfg-if", "pin-project-lite", + "tracing-attributes", "tracing-core", ] +[[package]] +name = "tracing-attributes" +version = "0.1.23" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4017f8f45139870ca7e672686113917c71c7a6e02d4924eda67186083c03081a" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "tracing-core" version = "0.1.30" @@ -2056,6 +2313,15 @@ dependencies = [ "unic-common", ] +[[package]] +name = "unicase" +version = "2.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "50f37be617794602aabbeee0be4f259dc1778fabe05e2d67ee8f79326d5cb4f6" +dependencies = [ + "version_check", +] + [[package]] name = "unicode-bidi" version = "0.3.10" @@ -2215,6 +2481,19 @@ version = "0.2.84" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0046fef7e28c3804e5e38bfa31ea2a0f73905319b677e57ebe37e49358989b5d" +[[package]] +name = "wasm-streams" +version = "0.2.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6bbae3363c08332cadccd13b67db371814cd214c2524020932f0804b8cf7c078" +dependencies = [ + "futures-util", + "js-sys", + "wasm-bindgen", + "wasm-bindgen-futures", + "web-sys", +] + [[package]] name = "web-sys" version = "0.3.61" diff --git a/Cargo.toml b/Cargo.toml index 2d62a99..d67ef02 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,6 +18,7 @@ path = "src/main.rs" [dependencies] anyhow = "1.0.69" +async-openai = "0.8.0" async-std = "1.12.0" async-trait = "0.1.64" clap = { version = "4.1.8", features = ["derive"] } diff --git a/src/llms/openai.rs b/src/llms/openai.rs index cda7c21..4d40c3e 100644 --- a/src/llms/openai.rs +++ b/src/llms/openai.rs @@ -1,20 +1,16 @@ -use std::time::Duration; - use anyhow::{anyhow, bail, Result}; use async_trait::async_trait; -use colored::Colorize; -use reqwest::{Client, ClientBuilder}; -use serde_json::{json, Value}; + use tiktoken_rs::tiktoken::{p50k_base, CoreBPE}; use crate::settings::OpenAISettings; +use async_openai::{types::CreateCompletionRequestArgs, Client}; use super::llm_client::LlmClient; #[derive(Clone, Debug)] pub(crate) struct OpenAIClient { - api_key: String, model: String, client: Client, } @@ -30,13 +26,8 @@ impl OpenAIClient { bail!("No OpenAI model configured.") } - let timeout = Duration::new(15, 0); - let client = ClientBuilder::new().timeout(timeout).build()?; - Ok(Self { - api_key, - model, - client, - }) + let client = Client::new().with_api_key(&api_key); + Ok(Self { model, client }) } pub(crate) fn get_prompt_token_limit_for_model(&self) -> usize { @@ -73,55 +64,31 @@ impl LlmClient for OpenAIClient { bail!(error_msg) } - let json_data = json!({ - "model": self.model, - "prompt": prompt, - "temperature": 0.5, - "max_tokens": output_length, - "top_p": 1, - "frequency_penalty": 0, - "presence_penalty": 0 - }); - - let request = self - .client - .post("https://api.openai.com/v1/completions") - .header("Content-Type", "application/json") - .header("Authorization", format!("Bearer {}", self.api_key)) - .json(&json_data); + // Create request using builder pattern + let request = CreateCompletionRequestArgs::default() + .model(&self.model) + .prompt(prompt) + .max_tokens(output_length as u16) + .temperature(0.5) + .top_p(1.) + .frequency_penalty(0.) + .presence_penalty(0.) + .build()?; debug!("Sending request to OpenAI:\n{:?}", request); - let response = request.send().await?; - let response_body = response.text().await?; - let json_response: Value = serde_json::from_str(&response_body).map_err(|e| { - anyhow!( - "Could not decode JSON response: {}\nResponse body: {:?}", - e, - response_body - ) - })?; - Ok(json_response["choices"][0]["text"] - .as_str() - .ok_or_else(|| { - let error_message: &str = json_response - .get("error") - .and_then(|e| e.get("message")) - .and_then(|m| m.as_str()) - .unwrap_or_default(); - if !error_message.is_empty() { - return anyhow!( - "{}", - format!("OpenAI error: {error_message}").bold().yellow() - ); - } + let response = self + .client + .completions() // Get the API "group" (completions, images, etc.) from the client + .create(request) // Make the API call in that "group" + .await?; + + let completion = response + .choices + .first() + .ok_or(anyhow!("No completion results")) + .map(|c| c.text.clone()); - anyhow!( - "Unexpected API response:\n{}", - json_response.to_string().yellow() - ) - })? - .trim() - .to_string()) + return completion; } } From 09376f237beec7bfdd4cb1c2c63672732a75941a Mon Sep 17 00:00:00 2001 From: Roger Zurawicki Date: Thu, 2 Mar 2023 18:42:54 -0500 Subject: [PATCH 3/3] Configure GPTCommit files for Unix systems - Create default configuration files for GPTcommit - Set permissions for the configuration files (Unix only) Closes #66 --- src/git.rs | 2 +- src/settings.rs | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/git.rs b/src/git.rs index 1dc276c..9f75628 100644 --- a/src/git.rs +++ b/src/git.rs @@ -43,7 +43,7 @@ pub(crate) fn get_hooks_path() -> Result { // create dirs first otherwise canonicalize will fail fs::create_dir_all(&rel_hooks_path)?; #[cfg(unix)] - fs::set_permissions(&rel_hooks_path, Permissions::from_mode(0o755))?; + fs::set_permissions(&rel_hooks_path, Permissions::from_mode(0o700))?; // turn relative path into absolute path let hooks_path = std::fs::canonicalize(rel_hooks_path)?; Ok(hooks_path) diff --git a/src/settings.rs b/src/settings.rs index 8761adb..84bd6e2 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -1,4 +1,6 @@ use std::{collections::HashMap, fs, path::PathBuf, str::FromStr}; +#[cfg(unix)] +use std::{fs::Permissions, os::unix::prelude::PermissionsExt}; use config::{ builder::DefaultState, Config, ConfigBuilder, ConfigError, Environment, File, Source, @@ -271,6 +273,8 @@ pub fn get_local_config_path() -> Option { .join("gptcommit.toml"); if !config_path.exists() { fs::write(&config_path, "").ok()?; + #[cfg(unix)] + fs::set_permissions(&config_path, Permissions::from_mode(0o600)).unwrap(); } return Some(config_path); } @@ -287,6 +291,8 @@ pub fn get_user_config_path() -> Option { let config_path = config_dir.join("config.toml"); if !config_path.exists() { fs::write(&config_path, "").ok()?; + #[cfg(unix)] + fs::set_permissions(&config_path, Permissions::from_mode(0o600)).unwrap(); } return Some(config_path); }