diff --git a/Cargo.toml b/Cargo.toml index 79c63fe..82ee677 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "cosmonaut_code" -version = "0.1.2" +version = "0.2.0" edition = "2021" license = "CC BY-NC-ND 4.0" readme = "README.md" @@ -38,10 +38,11 @@ sha2 = "0.10.8" handlebars = "4.5.0" # linguist-rs = "1.1.2" # Using direct repository fetch in place of crates.io as version is out of date (local code changes) linguist-rs = { git = "https://github.com/cosmonaut-nz/linguist-rs.git", version = "1.1.2" } +gcp_auth = "0.10.0" [dev-dependencies] tempfile = "3.8.1" [build-dependencies] -linguist-rs-build = { git = "https://github.com/cosmonaut-nz/linguist-rs.git", version = "1.1.1" } \ No newline at end of file +linguist-rs-build = { git = "https://github.com/cosmonaut-nz/linguist-rs.git", version = "1.1.1" } diff --git a/README.md b/README.md index f9fab1c..f4d4b31 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ it's a code explorer, explainer and assessment tool. to be honest, we built this tool because we needed it for the work we do. sure there are some great tools out there, but none of them quite hit the mark for our needs. -it's in pure rust! so it gotta be good, right? :roll_eyes: +generative ai will maximise outputs and 10x developer output. what about the quality? this tool aims to address this - i.e., use the source of the problem to provide a solution. ### goals @@ -16,7 +16,7 @@ it's in pure rust! so it gotta be good, right? :roll_eyes: 2. helps new developers to quickly get up to speed on a large or legacy codebase. 3. provide a tool that will help developers and code maintainers manage their code and start a conversation on quality. 4. allow code owners to check the overall health of the code in a simple way. -5. output an actionable report that will improve the code base. +5. output an actionable report with auto-PRs that will improve the code base over time. ### non-goals @@ -33,22 +33,37 @@ it's in pure rust! so it gotta be good, right? :roll_eyes: 5. entry-point for due diligence of technology assets. 6. code owner reporting on technical-debt and general health of asset. +## current state + +currently, most things are working and a solid report is produced in either `json` or `html`. + +the most stable and tested provider is openai. The best results, by far, are with the `gpt-4` service, which uses the latest `preview` model. the openai `gpt-3.5` works, but tends to over state issues and the quality of resolution offered to isses is not as good. it does run faster, however and is cheaper to run. + +google are late to the party, but have come in the door with a half-drunken bottle. the public instance of `gemini-pro` is both faster, cheaper and produces better results that openai's `gpt-3.5`. it is slightly behind the `gpt-4` `preview` model, but not far. do your own testing; we've found the late 2023 comparisons online to be highly misleading. the google provider is not as tested as openai, so you will see more errors in the log output. it should recover from these errors, but it is less robust. ## disclaimer this is really early days. running over a really big repo with the latest model will be super slow and possibly fail. we've tested it up to ~1500 code files, what with timeout retries etc., takes a couple of hours, cost about 5 usd. your mileage may vary. we think the value will come when it can be run over multiple models and compared and filtered. -it produces false flags. it overplays or (rarely downplays) security issues. there is significant variation between review runs on the same repository, particularly with older models. +as with all similar tools, it does produce false flags. it overplays or (rarely) downplays security issues. in some cases it may flag so many issues that the response is truncated, creating an error. we are working on this. + +there is significant variation between models and even review runs on the same repository with the same model, particularly with older models. some models are silent on obvious issues and transfixed on trivial issues. + +there are issues with the language file type matching via the github linguist regex. we will likely move to something more robust, or fix the crate that causes the mismatching. + +we recommend that you run it multiple times at first to gain a base line; fix the big issues and then let it run periodically. -right now it's a barebones offering. it works, and we have gotten value from it, but there is a lot more to do. but it's been fun to do. +right now it's deliberately a barebones offering. it works well, and we have gotten value from it, but there is a lot more to do. it's been fun to do. -use it as it is intended, as a start-point to a conversation on quality and current practices. +the google public api provider works, but is less robust than openai. + +there is a local instance wired up. it does work, but it highly fragile and unlikely to complete. it currently uses lm studio. ## usage download pre-release -[MacOS Apple Silicon](https://github.com/cosmonaut-nz/cosmonaut-code/releases/download/v0.1.2/cosmonaut_code_0.1.2_macos-aarch64) +[MacOS Apple Silicon](https://github.com/cosmonaut-nz/cosmonaut-code/releases/download/v0.2.0/cosmonaut_code_0.2.0_macos-aarch64) ### configuration @@ -58,33 +73,31 @@ configure: add a `settings.json`, maybe in the `settings` folder, with the follo { "sensitive": { - "api_key": "[YOUR_OPENAI_API_KEY]", - "org_id": "[YOUR_OPENAI_ORG_ID]", - "org_name": "[YOUR_OPENAI_ORG_NAME]" + "api_key": "[YOUR_API_KEY]" }, "repository_path": "[FULL_PATH_TO_REPO]", "report_output_path": "[FULL_PATH_TO_OUTPUT]", - "chosen_service": "gpt-4", - "output_type": "html", - "review_type": "general", + "chosen_provider": "[CHOICE OF PROVIDER]", + "chosen_service": "[CHOICE OF SERVICE]", + "output_type": "html" } ``` +`chosen_provider` is in: + +1. `openai` (default) +2. `google` (note API key only, ADC does not work as this is the public version) + `chosen_service` is in: 1. `gpt-4` (default) 2. `gpt-3.5` - -`review_type` is in: - -1. "general" = full review - (default) -2. "security" = security review only -3. "stats" = mock run, not using LLM for code review +3. `gemini-pro` (for google provider) `output_type` is in: -1. `HTML` +1. `html` 2. `json` - (default) run: @@ -94,11 +107,12 @@ run: export SENSITIVE_SETTINGS_PATH=[PATH_TO_YOUR_SETTINGS.JSON] ``` + download release above ```bash -mv cosmonaut_code_0.1.2_macos-aarch64 cosmonaut_code +mv cosmonaut_code_0.2.0_macos-aarch64 cosmonaut_code ``` @@ -110,7 +124,7 @@ mv cosmonaut_code_0.1.2_macos-aarch64 cosmonaut_code ## via rust locally -### tldr; +### tldr install rust; clone the repo; cd repo; add config (see above); `cargo run`. @@ -151,7 +165,7 @@ see [contributing](CONTRIBUTING.md) for the rules, they are standard though. we do our best to release working code. we hacked this out pretty quickly so the code's quality is not all that right now. -status today is: *"it works, but it is not that pretty or that user-friendly."* +status today is: *"it works, and the happy path is pretty solid. deviate from the path and there be dragons"* ## outline tasks @@ -159,17 +173,17 @@ status today is: *"it works, but it is not that pretty or that user-friendly."* - [X] enable open review of code - [X] output in json - [X] output in html -- [ ] packaging so user can either install via `cargo install` or download the binary -- [ ] output in pdf +- [X] packaging so user can either install via `cargo install` or download the binary (macos apple silicon only) - [X] (fine) tune the prompts for clarity and accuracy - [X] more configuration and adjustment of prompts -- [ ] github actions integration -- [ ] enable private llm review of code (likely llama-based) run on a cloud service +- [X] enable google gemini review of code +- [ ] enable a private google gemini review of code using vertex ai (coming soon) +- [ ] github actions integration (coming soon) +- [X] enable private llm review of code (likely llama-based) run on a cloud service. (not fully tested, but wired in to use lm studio) +- [ ] better collation of static data from `git` and the abstract source tree (ast) to feed the generative ai - [ ] proper documentation - [ ] gitlab pipeline integration -- [ ] enable google palm review of code -- [ ] enable anthropic claud review of code -- [ ] enable meta llama review of code +- [ ] make adding other providers easy and robust - e.g, a anthropic claud review of code - [ ] comparison of different llms review output on same code (this could be very cool!) `>_ we are cosmonaut` diff --git a/src/dev_mode/mod.rs b/src/dev_mode/mod.rs index 2bb9a5e..7dc08da 100644 --- a/src/dev_mode/mod.rs +++ b/src/dev_mode/mod.rs @@ -157,6 +157,7 @@ pub mod test_providers { content: _get_code_str(test_source_file)?, }], }; + info!("Prompt data: {:?}", prompt_data); let result = review_or_summarise(request_type, settings, provider, &prompt_data).await?; info!("Result: {:?}", result); Ok(()) @@ -187,6 +188,7 @@ pub mod test_providers { content: _get_code_str(test_source_file)?, }], }; + info!("Prompt data: {:#?}", prompt_data); let result = review_or_summarise(request_type, settings, provider, &prompt_data).await?; info!("Result: {:?}", result); diff --git a/src/provider/google.rs b/src/provider/google.rs index f17e7e8..7c2829d 100644 --- a/src/provider/google.rs +++ b/src/provider/google.rs @@ -1,24 +1,33 @@ //! The provider specific work for Google //! -use super::api::{ - ProviderCompletionResponse, ProviderResponseChoice, ProviderResponseConverter, - ProviderResponseMessage, -}; +use self::data::{GoogleCompletionResponse, GoogleResponseConverter}; +use super::api::{ProviderCompletionResponse, ProviderResponseConverter}; use super::{APIProvider, RequestType}; use crate::provider::prompts::PromptData; use crate::settings::{ProviderSettings, Settings}; -use std::time::Duration; - +// use gcp_auth::AuthenticationManager; // Only used for private API +use log::info; use reqwest::Client; -use serde::Deserialize; use serde_json::json; +use std::time::Duration; -/// What are the essential parts of a Google API request? +/// The Google public API provider works on the the following URL structure: /// - The API URL base - 'https://generativelanguage.googleapis.com/v1' /// - The 'models' extension - '/models', i.e., 'gemini-pro' /// - action: 'generateContent' -/// - overall: 'https://generativelanguage.googleapis.com/v1/models/gemini-pro:generateContent' +/// - example: 'https://generativelanguage.googleapis.com/v1/models/gemini-pro:generateContent' +/// +/// The Google private API provider works on the the following URL structure: +/// - The API URL base - 'https://us-central1-aiplatform.googleapis.com/v1' +/// - The 'projects' extension - '/projects', i.e., '[YOUR_PROJECT_ID]' +/// - The 'locations' extension - '/locations', i.e., 'us-central1' +/// - The 'publishers' extension - '/publishers', i.e., 'google' +/// - The 'models' extension - '/models', i.e., 'gemini-pro' +/// - action: 'streamGenerateContent' +/// - example: 'https://us-central1-aiplatform.googleapis.com/v1/projects/[YOUR_PROJECT_ID]/locations/us-central1/publishers/google/models/gemini-pro:streamGenerateContent' +/// +/// For public API use, the API key is required. For private API use, application default credentials (ADC) are required. pub(super) struct GoogleProvider { pub(super) model: String, } @@ -33,19 +42,29 @@ impl APIProvider for GoogleProvider { ) -> Result> { let provider: &ProviderSettings = settings.get_active_provider()?; + // set up GCP application default credentials (ADC) authn + // Uses URL: 'https://us-central1-aiplatform.googleapis.com/v1/projects/${PROJECT_ID}/locations/us-central1/publishers/google/models/${MODEL_ID}:streamGenerateContent' + // let authentication_manager = AuthenticationManager::new().await?; + // let scopes = &["https://www.googleapis.com/auth/cloud-platform"]; + // let token = authentication_manager.get_token(scopes).await?; + let client: Client = Client::builder() .timeout(Duration::from_secs(provider.api_timeout.unwrap_or(30))) .build()?; - let key = settings.sensitive.api_key.use_key(|key| key.to_string()); - - // TODO fix the API handling + // Extract out into a separate crate for the API handling, or if one arises, use that let google_url = format!( "{}/models/{}:generateContent?key={}", provider.api_url.clone(), self.model.clone(), - key + settings.sensitive.api_key.use_key(|key| key.to_string()) ); + // If this is a 'private' model, then we need to use the 'publishers' URL + // Note: the 'publishers' model does not provide the same response as the 'public' URL + // let google_url = + // format!("https://us-central1-aiplatform.googleapis.com/v1/projects/{}/locations/us-central1/publishers/google/models/{}:streamGenerateContent", + // "mickclarke138", + // self.model.clone()); let prompt_msgs: Vec = prompt_data .messages @@ -56,6 +75,7 @@ impl APIProvider for GoogleProvider { let response: Result = client .post(&google_url) .header(reqwest::header::USER_AGENT, env!("CARGO_CRATE_NAME")) + // .bearer_auth(&token.as_str().to_string()) // for private API only .header("Content-Type", "application/json") .json(&json!({ "contents": {"role": "user", "parts": prompt_msgs}, @@ -66,24 +86,33 @@ impl APIProvider for GoogleProvider { match response { Ok(res) => match res.status() { reqwest::StatusCode::OK => match res.json::().await { - Ok(data) => Ok(GoogleResponseConverter.to_generic_provider_response(&data)), - Err(e) => Err(format!("Failed to deserialize response: {}", e).into()), + Ok(mut data) => { + data.model = Some(self.model.clone()); + Ok(GoogleResponseConverter.to_generic_provider_response(&data)) + } + Err(e) => Err(format!( + "Failed to deserialize Google response into GoogleCompletionResponse: {}", + e + ) + .into()), }, reqwest::StatusCode::UNAUTHORIZED => { return Err(format!("Authorization error. Code: {:?}", res.status()).into()); } reqwest::StatusCode::BAD_REQUEST => { - return Err( - format!("Request not formed correctly. Code: {:?}", res.status()).into(), - ); - } - reqwest::StatusCode::FORBIDDEN => { return Err(format!( - "Forbidden. Check API permissions. Code: {:?}", + "API request format not correctly formed. Code: {:?}", res.status() ) .into()); } + reqwest::StatusCode::FORBIDDEN => { + let status = res.status(); + info!("Forbidden. Check API permissions. {:#?}", res.text().await); + return Err( + format!("Forbidden. Check API permissions. Code: {:?}", status).into(), + ); + } _ => { return Err( format!("An unexpected HTTP error code: . {:?}", res.status()).into(), @@ -92,92 +121,149 @@ impl APIProvider for GoogleProvider { }, Err(e) => { if e.is_timeout() { - Err("Network request timed out".into()) + Err("Google API server timed out".into()) } else if e.is_status() { - Err(format!("Server returned error: {}", e.status().unwrap()).into()) + Err(format!("Google API server returned error: {}", e.status().unwrap()).into()) } else { - Err(format!("Network request failed: {}", e).into()) + Err(format!("Google API request failed: {}", e).into()) } } } } } -// The structs that represent the Google API response -/// Top-level response from Google -#[derive(Debug, Deserialize)] -pub struct GoogleCompletionResponse { - pub candidates: Vec, - #[serde(rename = "promptFeedback")] - pub prompt_feedback: PromptFeedback, -} -/// Google has the concept of a candidate. Similar to an OpenAI 'choice', but at the top-level -#[derive(Debug, Deserialize)] -pub struct Candidate { - pub content: Content, - #[serde(rename = "finishReason")] - pub finish_reason: String, - pub index: i32, - #[serde(rename = "safetyRatings")] - pub safety_ratings: Vec, -} -/// The Content for a Candidate is further broken down into Parts -#[derive(Debug, Deserialize)] -pub struct Content { - pub parts: Vec, - pub role: GoogleRole, -} -#[derive(Debug, Deserialize)] -pub struct Part { - pub text: String, -} -/// Feedback on the prompt -#[derive(Debug, Deserialize)] -pub struct PromptFeedback { - #[serde(rename = "safetyRatings")] - pub safety_ratings: Vec, -} +/// The data structures for the Google API response +mod data { + use serde::Deserialize; -/// The Google safety rating -#[derive(Debug, Deserialize)] -pub struct SafetyRating { - pub category: String, - pub probability: String, -} -/// A Google role is only ever 'user' or 'agent' -#[derive(Debug, Deserialize)] -#[serde(rename_all = "lowercase")] -pub enum GoogleRole { - User, - Model, -} -// Implementation of ProviderResponseConverter for LM Studio. -pub(crate) struct GoogleResponseConverter; -impl ProviderResponseConverter for GoogleResponseConverter { - fn to_generic_provider_response( - &self, - google_response: &GoogleCompletionResponse, - ) -> ProviderCompletionResponse { - let mut messages: Vec = vec![]; - for candidate in &google_response.candidates { - for part in &candidate.content.parts { - messages.push(ProviderResponseMessage { - content: part.text.to_string(), - }); + use crate::provider::api::{ + ProviderCompletionResponse, ProviderResponseChoice, ProviderResponseConverter, + ProviderResponseMessage, + }; + + // The structs that represent the Google API response + /// Top-level response from Google + #[derive(Debug, Deserialize)] + pub struct GoogleCompletionResponse { + pub model: Option, + pub candidates: Vec, + #[serde(rename = "promptFeedback")] + pub prompt_feedback: PromptFeedback, + } + /// Google has the concept of a candidate. Similar to an OpenAI 'choice', but at the top-level + #[derive(Debug, Deserialize)] + pub struct Candidate { + pub content: Content, + #[serde(rename = "finishReason")] + pub finish_reason: String, + pub index: i32, + #[serde(rename = "safetyRatings")] + pub safety_ratings: Vec, + } + /// The Content for a Candidate is further broken down into Parts + #[derive(Debug, Deserialize)] + pub struct Content { + pub parts: Vec, + pub role: GoogleRole, + } + #[derive(Debug, Deserialize)] + pub struct Part { + pub text: String, + } + /// Feedback on the prompt + #[derive(Debug, Deserialize)] + pub struct PromptFeedback { + #[serde(rename = "safetyRatings")] + pub safety_ratings: Vec, + } + + /// The Google safety rating + #[derive(Debug, Deserialize)] + pub struct SafetyRating { + pub category: String, + pub probability: String, + } + /// A Google role is only ever 'user' or 'agent' + #[derive(Debug, Deserialize)] + #[serde(rename_all = "lowercase")] + pub enum GoogleRole { + User, + Model, + } + // Implementation of ProviderResponseConverter for LM Studio. + pub(crate) struct GoogleResponseConverter; + impl ProviderResponseConverter for GoogleResponseConverter { + fn to_generic_provider_response( + &self, + google_response: &GoogleCompletionResponse, + ) -> ProviderCompletionResponse { + let mut messages: Vec = vec![]; + for candidate in &google_response.candidates { + for part in &candidate.content.parts { + messages.push(ProviderResponseMessage { + content: part.text.to_string(), + }); + } + } + ProviderCompletionResponse { + id: "".to_string(), + model: google_response + .model + .clone() + .unwrap_or("none set".to_string()), + choices: vec![ProviderResponseChoice { + message: ProviderResponseMessage { + content: messages + .iter() + .map(|m| m.content.clone()) + .collect::>() + .join("\n"), + }, + }], } } - ProviderCompletionResponse { - id: "".to_string(), - model: "".to_string(), - choices: vec![ProviderResponseChoice { - message: ProviderResponseMessage { - content: messages - .iter() - .map(|m| m.content.clone()) - .collect::>() - .join("\n"), + } + + #[cfg(test)] + mod tests { + use super::*; + + #[test] + fn test_to_generic_provider_response() { + let google_response = GoogleCompletionResponse { + model: Some("gemini-pro".to_string()), + candidates: vec![Candidate { + content: Content { + parts: vec![ + Part { + text: "Hello".to_string(), + }, + Part { + text: "World".to_string(), + }, + ], + role: GoogleRole::User, + }, + finish_reason: "complete".to_string(), + index: 0, + safety_ratings: vec![SafetyRating { + category: "safety".to_string(), + probability: "high".to_string(), + }], + }], + prompt_feedback: PromptFeedback { + safety_ratings: vec![SafetyRating { + category: "safety".to_string(), + probability: "high".to_string(), + }], }, - }], + }; + + let converter = GoogleResponseConverter; + let provider_response = converter.to_generic_provider_response(&google_response); + + assert_eq!(provider_response.choices.len(), 1); + assert_eq!(provider_response.choices[0].message.content, "Hello\nWorld"); } } } diff --git a/src/provider/openai.rs b/src/provider/openai.rs index 59e58ef..0ebf97b 100644 --- a/src/provider/openai.rs +++ b/src/provider/openai.rs @@ -99,7 +99,6 @@ impl OpenAIProvider { continue; } } - info!("OpenAI API request failed from request: {:?}", req); return Err(format!("OpenAI API request failed: {}", openai_err).into()); } } diff --git a/src/provider/prompts/code_review.json b/src/provider/prompts/code_review.json index ec9aeba..b6b4254 100644 --- a/src/provider/prompts/code_review.json +++ b/src/provider/prompts/code_review.json @@ -6,7 +6,7 @@ }, { "role": "system", - "content": "As an expert code reviewer with comprehensive knowledge in software development standards, review the following code." + "content": "You are a code reviewer with comprehensive knowledge in software development standards." }, { "role": "system", @@ -14,11 +14,11 @@ }, { "role": "system", - "content": "Provide your analysis strictly in valid JSON format. Strictly escape any characters within your response strings that will create invalid JSON, such as \" - i.e., quotes - use a single escape character. Never use comments in your JSON. Ensure that your output exactly conforms to the following JSON Schema and you follow exactly the instructions provided in the 'description' fields." + "content": "Provide your analysis strictly in valid JSON format. Strictly escape any characters within your response strings that will create invalid JSON, such as \" - i.e., quotes - use a single escape character. Ensure you never leave trailing commas. Never use comments in your JSON. Ensure that your output exactly conforms to the following JSON Schema as provided. You MUST follow exactly the instructions provided in the 'description' fields. Ensure all 'required' fields have values; do not use 'null'." }, { "role": "system", - "content": "{{file_review_schema}}" + "content": "Exactly comply to the following JSON schema for your response: \n\n {{file_review_schema}}" } ] } \ No newline at end of file diff --git a/src/provider/specification/file_review.schema.json b/src/provider/specification/file_review.schema.json index cda350c..69bde4a 100644 --- a/src/provider/specification/file_review.schema.json +++ b/src/provider/specification/file_review.schema.json @@ -9,7 +9,7 @@ }, "file_rag_status": { "type": "string", - "description": "A RAG status for the file according to the findings", + "description": "A RAG status for the file according to the findings, strictly use enum values. Leave as null.", "enum": [ "Red", "Amber", @@ -57,11 +57,11 @@ }, "relative_path": { "type": "string", - "description": "The relative path of the file, including the file name and extension" + "description": "The relative path of the file, including the file name and extension. If no path, give the file name." }, "language": { "type": "languageType", - "description": "The details of the language the file is written in" + "description": "The details of the language the file is written in. Leave as null." }, "id_hash": { "type": "string", @@ -75,31 +75,7 @@ }, "required": [ "name", - "relative_path", - "id_hash", - "statistics" - ] - }, - "error": { - "type": "object", - "properties": { - "code": { - "type": "string", - "description": "Where in the code the error was found. Include line of code" - }, - "issue": { - "type": "string", - "description": "An error which will directly impact the function or performance of the code. An error is where the code is clearly non-adherent to language standards, best practice or clearly deviates from DRY or SOLID principles." - }, - "resolution": { - "type": "string", - "description": "A description of how the error can be resolved the error" - } - }, - "required": [ - "code", - "issue", - "resolution" + "relative_path" ] }, "securityIssue": { @@ -117,15 +93,15 @@ }, "code": { "type": "string", - "description": "Where in the code the issue was found. Include line of code" + "description": "Where in the code the issue was found. Include line of code, or state 'general' if the error is not specific to a line of code. Required, do not give null" }, "threat": { "type": "string", - "description": "A description of the threat or vulnerability, such as listed by OWASP, or CVE security vulnerability, etc. Detail the implications of the threat." + "description": "A description of the threat or vulnerability, such as listed by OWASP, or CVE security vulnerability, etc. Detail the implications of the threat. Required, do not give null" }, "mitigation": { "type": "string", - "description": "A description of how the threat can be mitigated" + "description": "A description of how the threat can be mitigated. Required, do not give null" } }, "required": [ @@ -135,20 +111,42 @@ "mitigation" ] }, + "error": { + "type": "object", + "properties": { + "code": { + "type": "string", + "description": "Where in the code the error was found. Include line of code, or state 'general' if the error is not specific to a line of code. Required, do not give null" + }, + "issue": { + "type": "string", + "description": "An error which will directly impact the function or performance of the code. An error is where the code is clearly non-adherent to language standards, best practice or clearly deviates from DRY or SOLID principles. Required, do not give null" + }, + "resolution": { + "type": "string", + "description": "A description of how the error can be resolved the error. Required, do not give null" + } + }, + "required": [ + "code", + "issue", + "resolution" + ] + }, "improvement": { "type": "object", "properties": { "code": { "type": "string", - "description": "Where in the code improvement can be made. Include the line of code as a snippet. Include the line number" + "description": "Where in the code improvement can be made. Include the line of code as a snippet. Include the line number, or state 'general' if the error is not specific to a line of code. Required, do not give null" }, "suggestion": { "type": "string", - "description": "A suggested improvement to the code, why and what can be done" + "description": "A suggested improvement to the code, why and what can be done. Required, do not give null" }, "improvement_details": { "type": "string", - "description": "Code that will make the improvement. Ensure the code is functionally complete and can be easily implemented in the source file you have reviewed. The code MUST align with the language of the source file" + "description": "Code that will make the improvement. Ensure the code is functionally complete and can be easily implemented in the source file you have reviewed. The code MUST align with the language of the source file. Required, do not give null" } }, "required": [ @@ -174,10 +172,7 @@ "description": "Statistics for this LanguageType. Leave as null" } }, - "required": [ - "name", - "extension" - ] + "required": [] }, "statistics": { "type": "object", diff --git a/src/retrieval/code.rs b/src/retrieval/code.rs index 117927b..88c5591 100644 --- a/src/retrieval/code.rs +++ b/src/retrieval/code.rs @@ -58,7 +58,8 @@ pub(crate) fn analyse_file_language(file_info: &mut SourceFileInfo) -> Option<&S if is_vendor_from_str(file_info.relative_path.clone(), &rules) || is_documentation_from_str(file_info.relative_path.clone(), &docs) || is_dotfile_from_str(file_info.relative_path.clone()) - || is_configuration_from_str(file_info.language.extension.clone()) + || file_info.language.is_some() + && is_configuration_from_str(file_info.language.as_ref().unwrap().extension.clone()) { // TODO: handle if is_documentation: if so then work out frequency; higher the count the better for overall RAG // if no documentation then needs to be in repository summary and flagged as issue @@ -69,8 +70,8 @@ pub(crate) fn analyse_file_language(file_info: &mut SourceFileInfo) -> Option<&S // Use the Linguist crate to determine the language - if is not a source file (i.e., not code) then return None let language: &Language = match resolve_language_from_content_str( file_info.get_source_file_contents(), - file_info.language.name.clone(), - file_info.language.extension.clone(), + file_info.language.as_ref().unwrap().name.clone(), + file_info.language.as_ref().unwrap().extension.clone(), &lc, ) { Ok(Some(lang)) => { @@ -98,7 +99,7 @@ pub(crate) fn analyse_file_language(file_info: &mut SourceFileInfo) -> Option<&S 0 } }; - file_info.language = LanguageType::from_language(language); // At this point we don't know whether there are other language types so we set the stats later + file_info.language = Some(LanguageType::from_language(language)); // At this point we don't know whether there are other language types so we set the stats later file_info.statistics.size = file_size; file_info.statistics.loc = loc; file_info.statistics.num_files += 1; diff --git a/src/retrieval/data.rs b/src/retrieval/data.rs index 20ed85d..485d109 100644 --- a/src/retrieval/data.rs +++ b/src/retrieval/data.rs @@ -154,7 +154,8 @@ impl LanguageType { pub(crate) struct SourceFileInfo { pub(crate) name: String, pub(crate) relative_path: String, - pub(crate) language: LanguageType, + #[serde(skip_serializing_if = "Option::is_none")] + pub(crate) language: Option, pub(crate) id_hash: Option, #[serde(skip)] pub(crate) source_file: Option>, @@ -172,7 +173,7 @@ impl SourceFileInfo { Self { name, relative_path, - language, + language: Some(language), id_hash: Some(id_hash), source_file: None, statistics, diff --git a/src/review/data.rs b/src/review/data.rs index 7feb05d..e257862 100644 --- a/src/review/data.rs +++ b/src/review/data.rs @@ -112,7 +112,8 @@ pub(crate) enum RAGStatus { pub(crate) struct SourceFileReview { pub(crate) source_file_info: SourceFileInfo, pub(crate) summary: String, - pub(crate) file_rag_status: RAGStatus, + #[serde(skip_serializing_if = "Option::is_none")] + pub(crate) file_rag_status: Option, #[serde(skip_serializing_if = "Option::is_none")] pub(crate) security_issues: Option>, #[serde(skip_serializing_if = "Option::is_none")] @@ -134,7 +135,7 @@ impl SourceFileReview { &self.improvements } #[allow(dead_code)] - pub(crate) fn get_file_rag_status(&self) -> &RAGStatus { + pub(crate) fn get_file_rag_status(&self) -> &Option { &self.file_rag_status } } @@ -246,7 +247,7 @@ mod tests { source_file_info: SourceFileInfo { name: "build.rs".to_string(), relative_path: "build.rs".to_string(), - language: LanguageType { + language: Some(LanguageType { name: "Rust".to_string(), extension: ".rs".to_string(), statistics: Some(Statistics { @@ -256,7 +257,7 @@ mod tests { num_commits: 0, frequency: 0.0, }), - }, + }), id_hash: Some("0".to_string()), source_file: None, statistics: Statistics { @@ -268,7 +269,7 @@ mod tests { }, }, summary: "This is a review summary".to_string(), - file_rag_status: RAGStatus::Green, + file_rag_status: Some(RAGStatus::Green), security_issues: Some(vec![SecurityIssue { severity: Severity::Low, code: "SEC001".to_string(), diff --git a/src/review/mod.rs b/src/review/mod.rs index 0f8a4dd..f5a554c 100644 --- a/src/review/mod.rs +++ b/src/review/mod.rs @@ -90,7 +90,7 @@ pub(crate) async fn assess_codebase( // Add the LanguageType to the Vec update_language_type_statistics(&mut lang_type_breakdown, &file_info); - let file_name_str = file_info.name.clone(); + let file_name_str = file_info.relative_path.clone(); let contents_str = file_info.get_source_file_contents(); // Actually review the file via the LLM, returns a SourceFileReview match review_file( @@ -141,7 +141,7 @@ fn update_language_type_statistics( ) { match lang_type_breakdown .iter() - .position(|lang| lang.name == file_info.language.name) + .position(|lang| lang.name == file_info.language.as_ref().unwrap().name) { Some(index) => { let language_stats = lang_type_breakdown @@ -154,9 +154,16 @@ fn update_language_type_statistics( } } None => { - let mut new_lang_type = file_info.language.clone(); - new_lang_type.statistics = Some(file_info.statistics.clone()); - lang_type_breakdown.push(new_lang_type); + if let Some(mut new_lang_type) = file_info.language.clone() { + if let Some(statistics) = &mut new_lang_type.statistics { + statistics.size += file_info.statistics.size; + statistics.loc += file_info.statistics.loc; + statistics.num_files += 1; + } else { + new_lang_type.statistics = Some(file_info.statistics.clone()); + lang_type_breakdown.push(new_lang_type); + } + } } } } @@ -217,7 +224,7 @@ fn update_review_summary(review_summary: &mut ReviewSummary, reviewed_file: &mut review_summary.text.push('\n'); reviewed_file.file_rag_status = - calculate_rag_status_for_reviewed_file(reviewed_file).unwrap_or_default(); + Some(calculate_rag_status_for_reviewed_file(reviewed_file).unwrap_or_default()); } /// Finalise the [`RepositoryReview`] by adding the [`ReviewSummary`], Vec, and other data @@ -277,8 +284,10 @@ async fn review_file( if let Some(mut prompt_data) = get_prompt_data_based_on_review_type(settings)? { let provider: &ProviderSettings = get_provider(settings); - let review_request: String = - format!("File name: {}\n{}\n", code_file_path, code_file_contents); + let review_request: String = format!( + "Source file to review:\n file name: {}\n contents: \n{}\n", + code_file_path, code_file_contents + ); prompt_data.add_user_message_prompt(review_request); perform_review(settings, provider, &prompt_data).await @@ -323,7 +332,13 @@ async fn perform_review( error!("Error in review: {}", e); attempts += 1; } - Err(e) => return Err(e), + Err(e) => { + error!( + "Failed after {} attempts. Cannot recover from error ", + max_retries + ); + return Err(e); + } } } } @@ -341,9 +356,9 @@ fn process_llm_response( }) .and_then(|stripped_json| { data::deserialize_file_review(&stripped_json).map_err(|e| { - error!( - "Failed to deserialize: {:?}, Possibly due to invalid escape character", - &stripped_json + warn!( + "Failed to deserialize ProviderCompletionResponse from model: {}; JSON: {:?}", + response.model, &stripped_json ); Box::new(e) as Box }) diff --git a/src/settings/mod.rs b/src/settings/mod.rs index b18534f..1a6360c 100644 --- a/src/settings/mod.rs +++ b/src/settings/mod.rs @@ -22,7 +22,9 @@ pub(crate) struct Settings { #[serde(skip_serializing_if = "Option::is_none")] pub(crate) chosen_service: Option, pub(crate) default_provider: String, + #[serde(default)] pub(crate) output_type: OutputType, + #[serde(default)] pub(crate) review_type: ReviewType, #[serde(skip_serializing_if = "Option::is_none")] pub(crate) review_cycles: Option,