Skip to content

Commit

Permalink
Merge pull request #1641 from codestoryai/features/fix-relative-file-…
Browse files Browse the repository at this point in the history 
…paths-problem

[sidecar] fix absolute path issues
  • Loading branch information
@theskcd
theskcd authored Dec 17, 2024
2 parents 44bf969 + 14d0d91 commit 1094ab2
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 284 deletions.
19 changes: 19 additions & 0 deletions reproduce_error.py
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import subprocess
import sys

def main():
# Try to list available tools in zed
try:
result = subprocess.run(['cargo', 'run', '--bin', 'sidecar', '--', 'tools', 'list'],
cwd='sidecar',
capture_output=True,
text=True)
print("Current tools available:")
print(result.stdout)
print("\nError output:")
print(result.stderr)
except Exception as e:
print(f"Error running sidecar: {e}")

if __name__ == "__main__":
main()
14 changes: 4 additions & 10 deletions sidecar/src/agentic/tool/session/service.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ impl SessionService {
// we should ideally get this information from the vscode-server side setting
let tool_agent = ToolUseAgent::new(
llm_broker.clone(),
root_directory,
root_directory.to_owned(),
std::env::consts::OS.to_owned(),
shell.to_owned(),
Some(repo_name),
Expand Down Expand Up @@ -463,10 +463,7 @@ impl SessionService {
tool_type.clone(),
tool_input_partial,
tool_box.clone(),
tool_agent.clone(),
user_message.to_owned(),
false,
true,
root_directory.to_owned(),
message_properties.clone(),
)
.await;
Expand Down Expand Up @@ -563,7 +560,7 @@ impl SessionService {
// we should ideally get this information from the vscode-server side setting
let tool_agent = ToolUseAgent::new(
llm_broker.clone(),
root_directory,
root_directory.to_owned(),
std::env::consts::OS.to_owned(),
shell.to_owned(),
None,
Expand Down Expand Up @@ -629,10 +626,7 @@ impl SessionService {
tool_type.clone(),
tool_input_partial,
tool_box.clone(),
tool_agent.clone(),
user_message.to_owned(),
false, // is not part of test genertaion
false,
root_directory.to_owned(),
message_properties.clone(),
)
.await?;
Expand Down
285 changes: 11 additions & 274 deletions sidecar/src/agentic/tool/session/session.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -704,62 +704,6 @@ impl Session {
.find(|exchange| &exchange.exchange_id == exchange_id)
}

fn decay_messages(
&self,
// both of these have the same length
exchanges: &[Exchange],
mut conversation_messages: Vec<SessionChatMessage>,
) -> Vec<SessionChatMessage> {
// The algorithm we use for decay is the following:
// - When using any tool which is of map type query -> List<Results>
// we keep the tool output for the map tool type as long as there is not
// a mutation (code_edit) or we use another map type tool
// - This allows us to keep the token usage small while still retaining
let mut previous_map_tool_indices = vec![];
for (idx, exchange) in exchanges.into_iter().enumerate() {
match &exchange.exchange_type {
ExchangeType::AgentChat(agent_chat) => match &agent_chat.reply {
ExchangeReplyAgent::Tool(tool_input) => {
let input_tool_type = &tool_input.tool_type;
// We are have an input tool over here
// map_tool_type || mutation_tool_type
// [T T T T M I I C T M I I I M M T T]
// if we get a C which is code-edit
// then we remove the output of the previous map test
// and if we have multiple M steps we remove all of them
// until we get a C (code-edit)
if input_tool_type.is_map_type() {
previous_map_tool_indices.push(idx);
}
if input_tool_type.is_code_edit_type() {
// rest all the running map types over here sinc we
// have started started code editing
previous_map_tool_indices.into_iter().for_each(
|map_tool_input_index| {
// the tool output is generally immediately
// after the current tool input index
let role = conversation_messages[map_tool_input_index + 1]
.role()
.clone();
conversation_messages[map_tool_input_index + 1] =
SessionChatMessage::new(
role,
"... truncated output".to_owned(),
vec![],
);
},
);
previous_map_tool_indices = vec![];
}
}
_ => {}
},
_ => {}
}
}
conversation_messages
}

/// Finds the exchange we are interested in and mutates the previous queries
/// and the current query
pub fn plan_iteration(
Expand Down Expand Up @@ -2110,95 +2054,14 @@ impl Session {
self
}

async fn handle_critique(
mut self,
tool_agent: ToolUseAgent,
tool_box: Arc<ToolBox>,
original_user_message: String,
message_properties: SymbolEventMessageProperties,
) -> Result<Self, SymbolError> {
// figure out what to do over here given the state of the session
let mut converted_messages = vec![];
for previous_message in self.exchanges.iter() {
converted_messages.push(
previous_message
.to_conversation_message(tool_box.tools().clone())
.await,
);
}

// decay the content of the messages depending on the decay condition
// so we can keep the context smaller and more relevant
converted_messages = self.decay_messages(self.exchanges.as_slice(), converted_messages);
let input =
ToolUseAgentInput::new(converted_messages, vec![], None, message_properties.clone());
let critique = tool_agent.invoke_critique(input).await?;
// reset all the exchanges since we are going to start a new
println!("session::handle_critique::starting_new");
let exchange_message = format!(
r#"When trying to solve this issue before we ran into a wrong approach, the patch was reviewed by a senior engineer who had the following helpful feedback to share:
{}
We have also reset the repository state and discarded all the changes which you did before. This is to help you start on a fresh plate.
The Github Issue we are trying to solve is:
{original_user_message}"#,
critique
);
self.exchanges = vec![];
self.exchanges.push(Exchange::human_chat(
"critique".to_owned(),
exchange_message,
UserContext::default(),
self.project_labels.to_vec(),
self.repo_ref.clone(),
));

// reset the repository status by running git stash
let _ = tool_box
.use_terminal_command("git stash", message_properties)
.await;
self.save_to_storage().await?;
Ok(self)
}

pub async fn invoke_tool(
mut self,
tool_type: ToolType,
tool_input_partial: ToolInputPartial,
tool_box: Arc<ToolBox>,
tool_agent: ToolUseAgent,
original_user_message: String,
is_test_generation: bool,
is_swe_bench: bool,
root_directory: String,
message_properties: SymbolEventMessageProperties,
) -> Result<Self, SymbolError> {
// we want to send a new event only when we are not going to ask for the followup questions
// we might have generated a new exchange id over here if we are going to be working
// on top of any tool which does not require user feedback
// let exchange_id = if !matches!(tool_type, ToolType::AskFollowupQuestions)
// && !matches!(tool_type, ToolType::AttemptCompletion)
// {
// let new_exchange_id = tool_box
// .create_new_exchange(
// message_properties.root_request_id().to_owned(),
// message_properties.clone(),
// )
// .await?;
// message_properties = message_properties.set_request_id(new_exchange_id.to_owned());
// let session_id = message_properties.root_request_id().to_owned();
// let exchange_id = message_properties.request_id_str().to_owned();
// let _ = message_properties
// .ui_sender()
// .send(UIEventWithID::tool_output_type_found(
// session_id.to_owned(),
// exchange_id.to_owned(),
// tool_type.clone(),
// ));
// new_exchange_id
// } else {
// message_properties.request_id_str().to_owned()
// };
let exchange_id = message_properties.request_id_str().to_owned();
match tool_input_partial {
ToolInputPartial::TestRunner(test_runner) => {
Expand Down Expand Up @@ -2247,24 +2110,6 @@ The Github Issue we are trying to solve is:
formatted_output, // truncated
UserContext::default(),
);

// The test running is a terminal condition, if we have an exit code 0
// we are good, otherwise the cirtique will take a look
// we only do this when we are not generating test cases
if !is_test_generation {
if test_runner_output.exit_code() != 0 {
return self
.handle_critique(
tool_agent,
tool_box,
original_user_message,
message_properties,
)
.await;
} else if test_runner_output.exit_code() == 0 {
return Err(SymbolError::TestCaseIsPassing);
}
}
}
ToolInputPartial::AskFollowupQuestions(_followup_question) => {
// this waits for the user-feedback so we do not need to react or
Expand All @@ -2278,7 +2123,15 @@ The Github Issue we are trying to solve is:
// figure out what to do over here
}
ToolInputPartial::CodeEditing(code_editing) => {
let fs_file_path = code_editing.fs_file_path().to_owned();
let mut fs_file_path = code_editing.fs_file_path().to_owned();
if !std::path::Path::new(&fs_file_path).is_absolute() {
// we need to join the file path here with the root directory
println!("fixing relative file path");
fs_file_path = std::path::Path::new(&root_directory)
.join(fs_file_path)
.to_string_lossy()
.to_string();
}
println!("Code editing: {}", fs_file_path);
let file_contents = tool_box
.file_open(fs_file_path.to_owned(), message_properties.clone())
Expand All @@ -2294,123 +2147,7 @@ The Github Issue we are trying to solve is:

// if the file is very very large then we chunk it up and use search and replace
// on individual chunks instead
let _ = if file_contents.lines().into_iter().collect::<Vec<_>>().len()
>= 1300
// if we are not in swe_bench mode, never try to be extra, go with
// the standard search and replace flow
&& !is_swe_bench
{
let first_part_lines = file_contents
.to_owned()
.lines()
.into_iter()
.enumerate()
.filter_map(|(idx, line)| {
if idx <= 750 {
Some(line.to_owned())
} else {
None
}
})
.collect::<Vec<_>>()
.join("\n");
let second_part_lines = file_contents
.to_owned()
.lines()
.into_iter()
.enumerate()
.filter_map(|(idx, line)| {
if idx > 750 {
Some(line.to_owned())
} else {
None
}
})
.collect::<Vec<_>>()
.join("\n");
let first_range =
Range::new(Position::new(0, 0, 0), Position::new(10_000, 0, 0));
let second_range =
Range::new(Position::new(0, 0, 0), Position::new(10_000, 0, 0));

// First half of the file has been edited
let symbol_to_edit = SymbolToEdit::new(
fs_file_path.to_owned(),
first_range,
fs_file_path.to_owned(),
vec![instruction.clone()],
false,
false, // is_new
false,
"".to_owned(),
None,
false,
None,
false,
None,
vec![], // previous_user_queries
None,
);

let symbol_identifier = SymbolIdentifier::new_symbol(&fs_file_path);

let first_part_edited = tool_box
.code_editing_with_search_and_replace(
&symbol_to_edit,
&fs_file_path,
&first_part_lines,
&first_range,
"".to_owned(),
instruction.clone(),
&symbol_identifier,
None,
None,
message_properties.clone(),
)
.await?; // big expectations but can also fail, we should handle it properly

// Editing second half of the file
let symbol_to_edit = SymbolToEdit::new(
fs_file_path.to_owned(),
second_range,
fs_file_path.to_owned(),
vec![instruction.clone()],
false,
false, // is_new
false,
"".to_owned(),
None,
false,
None,
false,
None,
vec![], // previous_user_queries
None,
);

let symbol_identifier = SymbolIdentifier::new_symbol(&fs_file_path);

let second_part_edited = tool_box
.code_editing_with_search_and_replace(
&symbol_to_edit,
&fs_file_path,
&second_part_lines,
&second_range,
"".to_owned(),
format!(r#"{}
This is part of the file which might not contain the method in full, if thats the case do not generate any edits"#, instruction.clone()),
&symbol_identifier,
None,
None,
message_properties.clone(),
)
.await?; // big expectations but can also fail, we should handle it properly
format!(
r#"{}
{}"#,
first_part_edited, second_part_edited
)
} else {
let _ = {
let default_range =
// very large end position
Range::new(Position::new(0, 0, 0), Position::new(10_000, 0, 0));
Expand Down

0 comments on commit 1094ab2

Please sign in to comment.