Yehowshua/ZIm
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Have models indicate code locations in workflows using textual search, not symbol names (#17282)

Browse source 
Release Notes:

- N/A

---------

Co-authored-by: Antonio Scandurra <me@as-cii.com>
This commit is contained in:
Max Brunsfeld 2024-09-02 18:20:05 -07:00 • committed by GitHub
parent c63c2015a6
commit b41ddbd018
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 410 additions and 820 deletions
Download patch file Download diff file Expand all files Collapse all files

650
assets/prompts/edit_workflow.hbs
View file

@ -27,17 +27,17 @@ impl Person {
``` ```
<edit> <edit>
<path>src/person.rs</path> <path>src/person.rs</path>
<operation>insert_before</operation> <operation>insert_before</operation>
<symbol>struct Person height</symbol> <search>height: f32,</search>
<description>Add the age field</description> <description>Add the age field</description>
</edit> </edit>
<edit> <edit>
<path>src/person.rs</path> <path>src/person.rs</path>
<operation>append_child</operation> <operation>insert_after</operation>
<symbol>impl Person</symbol> <search>impl Person {</search>
<description>Add the age getter</description> <description>Add the age getter</description>
</edit> </edit>
</step> </step>
@ -45,15 +45,15 @@ impl Person {
First, each `<step>` must contain a written description of the change that should be made. The description should begin with a high-level overview, and can contain markdown code blocks as well. The description should be self-contained and actionable. First, each `<step>` must contain a written description of the change that should be made. The description should begin with a high-level overview, and can contain markdown code blocks as well. The description should be self-contained and actionable.
Each `<step>` must contain one or more `<edit>` tags, each of which refer to a specific range in a source file. Each `<edit>` tag must contain the following child tags: After the description, each `<step>` must contain one or more `<edit>` tags, each of which refer to a specific range in a source file. Each `<edit>` tag must contain the following child tags:
### `<path>` (required) ### `<path>` (required)
This tag contains the path to the file that will be changed. It can be an existing path, or a path that should be created. This tag contains the path to the file that will be changed. It can be an existing path, or a path that should be created.
### `<symbol>` (optional) ### `<search>` (optional)
This tag contains the fully-qualified name of a symbol in the source file, e.g. `mod foo impl Bar pub fn baz` instead of just `fn baz`. If not provided, the new content will be inserted at the top of the file. This tag contains a search string to locate in the source file, e.g. `pub fn baz() {`. If not provided, the new content will be inserted at the top of the file. Make sure to produce a string that exists in the source file and that isn't ambiguous. When there's ambiguity, add more lines to the search to eliminate it.
### `<description>` (required) ### `<description>` (required)
@ -62,110 +62,179 @@ This tag contains a single-line description of the edit that should be made at t
### `<operation>` (required) ### `<operation>` (required)
This tag indicates what type of change should be made, relative to the given location. It can be one of the following: This tag indicates what type of change should be made, relative to the given location. It can be one of the following:
- `update`: Rewrites the specified symbol entirely based on the given description. - `update`: Rewrites the specified string entirely based on the given description.
- `create`: Creates a new file with the given path based on the provided description. - `create`: Creates a new file with the given path based on the provided description.
- `insert_sibling_before`: Inserts a new symbol based on the given description as a sibling before the specified symbol. - `insert_before`: Inserts new text based on the given description before the specified search string.
- `insert_sibling_after`: Inserts a new symbol based on the given description as a sibling after the specified symbol. - `insert_after`: Inserts new text based on the given description after the specified search string.
- `prepend_child`: Inserts a new symbol as a child of the specified symbol at the start. - `delete`: Deletes the specified string from the containing file.
- `append_child`: Inserts a new symbol as a child of the specified symbol at the end.
- `delete`: Deletes the specified symbol from the containing file.
<guidelines> <guidelines>
- There's no need to describe *what* to do, just *where* to do it. - There's no need to describe *what* to do, just *where* to do it.
- Only reference locations that actually exist (unless you're creating a file). - Only reference locations that actually exist (unless you're creating a file).
- If creating a file, assume any subsequent updates are included at the time of creation. - If creating a file, assume any subsequent updates are included at the time of creation.
- Don't create and then update a file. Always create new files in shot. - Don't create and then update a file. Always create new files in one hot.
- Prefer updating symbols lower in the syntax tree if possible. - Prefer multiple edits to smaller regions, as opposed to one big edit to a larger region.
- Never include edits on a parent symbol and one of its children in the same edit block. - Don't produce edits that intersect each other. In that case, merge them into a bigger edit.
- Never nest an edit with another edit. Never include CDATA. All edits are leaf nodes. - Never nest an edit with another edit. Never include CDATA. All edits are leaf nodes.
- Descriptions are required for all edits except delete. - Descriptions are required for all edits except delete.
- When generating multiple edits, ensure the descriptions are specific to each individual operation. - When generating multiple edits, ensure the descriptions are specific to each individual operation.
- Avoid referring to the location in the description. Focus on the change to be made, not the location where it's made. That's implicit with the symbol you provide. - Avoid referring to the search string in the description. Focus on the change to be made, not the location where it's made. That's implicit with the `search` string you provide.
- Don't generate multiple edits at the same location. Instead, combine them together in a single edit with a succinct combined description. - Don't generate multiple edits at the same location. Instead, combine them together in a single edit with a succinct combined description.
- Always ensure imports are added if you're referencing symbols that are not in scope. To manipulate imports, produce an edit where the `"symbol"` key is set to `"#imports"` - Always ensure imports are added if you're referencing symbols that are not in scope.
</guidelines> </guidelines>
Here are some concrete examples. Here are some concrete examples.
<example language="rust"> <example>
<message role="user"> <message role="user">
```rs src/rectangle.rs
struct Rectangle { ```rs src/shapes.rs
pub mod rectangle;
pub mod circle;
```
```rs src/shapes/rectangle.rs
pub struct Rectangle {
width: f64, width: f64,
height: f64, height: f64,
} }
impl Rectangle { impl Rectangle {
fn new(width: f64, height: f64) -> Self { pub fn new(width: f64, height: f64) -> Self {
Rectangle { width, height } Rectangle { width, height }
} }
} }
``` ```
We need to add methods to calculate the area and perimeter of the rectangle. Can you help with that? ```rs src/shapes/circle.rs
pub struct Circle {
radius: f64,
}
impl Circle {
pub fn new(radius: f64) -> Self {
Circle { radius }
}
}
```
Update all shapes to store their origin as an (x, y) tuple and implement Display.
</message> </message>
<message role="assistant"> <message role="assistant">
We'll need to update both the rectangle and circle modules.
<step> <step>
Add methods to calculate the area and perimeter of the rectangle Add origin fields to both shape types.
```rust ```rust
impl Rectangle { struct Rectangle {
// existing methods... // existing fields ...
origin: (f64, f64),
}
```
fn calculate_area(&self) -> f64 { ```rust
self.width * self.height struct Circle {
} // existing fields ...
origin: (f64, f64),
fn calculate_perimeter(&self) -> f64 {
2.0 * (self.width + self.height)
}
} }
``` ```
<edit> <edit>
<path>src/rectangle.rs</path> <path>src/shapes/rectangle.rs</path>
<operation>append_child</operation> <operation>insert_before</operation>
<symbol>impl Rectangle</symbol> <search>
<description>Add calculate_area and calculate_perimeter methods</description> width: f64,
height: f64,
</search>
<description>Add the origin field to Rectangle</description>
</edit>
<edit>
<path>src/shapes/circle.rs</path>
<operation>insert_before</operation>
<search>
radius: f64,
</search>
<description>Add the origin field to Circle</description>
</edit>
<step>
Update both shape's constructors to take an origin.
<edit>
<path>src/shapes/rectangle.rs</path>
<operation>update</operation>
<search>
fn new(width: f64, height: f64) -> Self {
Rectangle { width, height }
}
</search>
<description>Update the Rectangle new function to take an origin</description>
</edit>
<edit>
<path>src/shapes/circle.rs</path>
<operation>update</operation>
<search>
fn new(radius: f64) -> Self {
Circle { radius }
}
</search>
<description>Update the Circle new function to take an origin</description>
</edit> </edit>
</step> </step>
<step> <step>
Implement the Display trait for the Rectangle struct Implement Display for both shapes
```rust
use std::fmt;
impl fmt::Display for Rectangle {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "Rectangle: {}x{}", self.width, self.height)
}
}
```
<edit> <edit>
<path>src/rectangle.rs</path> <path>src/shapes/rectangle.rs</path>
<operation>insert_sibling_after</operation> <operation>insert_before</operation>
<symbol>impl Rectangle</symbol> <search>
<description>Implement Display trait for Rectangle</description> struct Rectangle {
</search>
<description>Add an import for the `std::fmt` module</description>
</edit> </edit>
<edit> <edit>
<path>src/rectangle.rs</path> <path>src/shapes/rectangle.rs</path>
<operation>append_child</operation> <operation>insert_after</operation>
<symbol>#imports</symbol> <search>
<description>Add use statement for fmt</description> Rectangle { width, height }
}
}
</search>
<description>Add a Display implementation for Rectangle</description>
</edit>
<edit>
<path>src/shapes/circle.rs</path>
<operation>insert_before</operation>
<search>
struct Circle {
</search>
<description>Add an import for the `std::fmt` module</description>
</edit>
<edit>
<path>src/shapes/circle.rs</path>
<operation>insert_after</operation>
<search>
Circle { radius }
}
}
</search>
<description>Add a Display implementation for Circle</description>
</edit> </edit>
</step> </step>
</message> </message>
</example> </example>
<example language="rust"> <example number=2>
<message role="user"> <message role="user">
```rs src/user.rs ```rs src/user.rs
struct User { struct User {
pub name: String, pub name: String,
@ -202,10 +271,14 @@ impl User {
``` ```
<edit> <edit>
<path>src/user.rs</path> <path>src/user.rs</path>
<operation>update</operation> <operation>update</operation>
<symbol>impl User pub fn print_info</symbol> <search>
<description>Print all the user information</description> pub fn print_info(&self) {
todo!()
}
</search>
<description>Print all the user information</description>
</edit> </edit>
</step> </step>
@ -213,442 +286,25 @@ impl User {
Remove the 'email' field from the User struct Remove the 'email' field from the User struct
<edit> <edit>
<path>src/user.rs</path> <path>src/user.rs</path>
<operation>delete</operation> <operation>delete</operation>
<symbol>struct User email</symbol> <search>
email: String,
</search>
</edit> </edit>
<edit> <edit>
<path>src/user.rs</path> <path>src/user.rs</path>
<operation>update</operation> <operation>update</operation>
<symbol>impl User fn new</symbol> <symbol>
<description>Remove email parameter from new method</description> fn new(name: String, age: u32, email: String) -> Self {
</edit> User { name, age, email }
</step>
</message>
</example>
<example language="rust">
<message role="user">
```rs src/vehicle.rs
struct Vehicle {
make: String,
model: String,
year: u32,
} }
</symbol>
impl Vehicle { <description>Remove email parameter from new method</description>
fn new(make: String, model: String, year: u32) -> Self {
Vehicle { make, model, year }
}
fn print_year(&self) {
println!("Year: {}", self.year);
}
}
```
Let's add a `start_engine` method for the vehicle and print how long it takes to start it.
</message>
<message role="assistant">
<step>
Add a 'use std::time::Instant;' statement at the beginning of the file
<edit>
<path>src/vehicle.rs</path>
<operation>prepend_child</operation>
<symbol>#imports</symbol>
<description>Add use statement for std::time::Instant</description>
</edit>
</step>
<step>
Add a new method 'start_engine' in the Vehicle impl block
```rust
impl Vehicle {
// ... existing methods ...
fn start_engine(&self) {
let t0 = Instant::now();
println!("Starting the engine of {} {} {}. Took: {:?}",
self.year, self.make, self.model, t0.elapsed());
}
}
```
<edit>
<path>src/vehicle.rs</path>
<operation>append_child</operation>
<symbol>impl Vehicle</symbol>
<description>Add start_engine method</description>
</edit> </edit>
</step> </step>
</message> </message>
</example>
<example language="rust">
<message role="user">
```rs src/employee.rs
struct Employee {
name: String,
position: String,
salary: u32,
department: String,
}
impl Employee {
fn new(name: String, position: String, salary: u32, department: String) -> Self {
Employee { name, position, salary, department }
}
fn print_details(&self) {
println!("Name: {}, Position: {}, Salary: {}, Department: {}",
self.name, self.position, self.salary, self.department);
}
fn give_raise(&mut self, amount: u32) {
self.salary += amount;
}
}
```
Let's turn the salary field into an f32. Also, let's delete the department field.
</message>
<message role="assistant">
<step>
Make salary an f32
```rust
struct Employee {
name: String,
position: String,
salary: f32,
department: String,
}
impl Employee {
fn new(name: String, position: String, salary: f32, department: String) -> Self {
Employee { name, position, salary, department }
}
// ... other methods ...
fn give_raise(&mut self, amount: f32) {
self.salary += amount;
}
}
```
<edit>
<path>src/employee.rs</path>
<operation>update</operation>
<symbol>struct Employee salary</symbol>
<description>Change salary type to f32</description>
</edit>
<edit>
<path>src/employee.rs</path>
<operation>update</operation>
<symbol>impl Employee fn new</symbol>
<description>Update new method to use f32 for salary</description>
</edit>
<edit>
<path>src/employee.rs</path>
<operation>update</operation>
<symbol>impl Employee fn give_raise</symbol>
<description>Update give_raise method to use f32 for amount</description>
</edit>
</step>
<step>
Remove the 'department' field and update the 'print_details' method
```rust
struct Employee {
name: String,
position: String,
salary: f32,
}
impl Employee {
fn new(name: String, position: String, salary: f32) -> Self {
Employee { name, position, salary }
}
fn print_details(&self) {
println!("Name: {}, Position: {}, Salary: {}",
self.name, self.position, self.salary);
}
// ... other methods ...
}
```
<edit>
<path>src/employee.rs</path>
<operation>delete</operation>
<symbol>struct Employee department</symbol>
</edit>
<edit>
<path>src/employee.rs</path>
<operation>update</operation>
<symbol>impl Employee fn new</symbol>
<description>Remove department parameter from new method</description>
</edit>
<edit>
<path>src/employee.rs</path>
<operation>update</operation>
<symbol>impl Employee fn print_details</symbol>
<description>Remove department from print_details method</description>
</edit>
</step>
</message>
</example>
<example language="rust">
<message role="user">
```rs src/game.rs
struct Player {
name: String,
health: i32,
pub score: u32,
}
impl Player {
pub fn new(name: String) -> Self {
Player { name, health: 100, score: 0 }
}
}
struct Game {
players: Vec<Player>,
}
impl Game {
fn new() -> Self {
Game { players: Vec::new() }
}
}
```
Introduce a level field.
</message>
<message role="assistant">
<step>
Add a 'level' field to Player and update the 'new' method
```rust
struct Player {
name: String,
health: i32,
pub score: u32,
level: u8,
}
impl Player {
pub fn new(name: String) -> Self {
Player { name, health: 100, score: 0, level: 1 }
}
}
```
<edit>
<path>src/game.rs</path>
<operation>insert_sibling_after</operation>
<symbol>struct Player pub score</symbol>
<description>Add level field to Player struct</description>
</edit>
<edit>
<path>src/game.rs</path>
<operation>update</operation>
<symbol>impl Player pub fn new</symbol>
<description>Initialize level in new method</description>
</edit>
</step>
</message>
</example>
<example language="rust">
<message role="user">
```rs src/config.rs
use std::collections::HashMap;
struct Config {
settings: HashMap<String, String>,
}
impl Config {
fn new() -> Self {
Config { settings: HashMap::new() }
}
}
```
I want to load the configuration from a file.
</message>
<message role="assistant">
<step>
Add a 'load_from_file' method to Config and import necessary modules
```rust
use std::collections::HashMap;
use std::fs::File;
use std::io::{self, BufRead};
use std::path::Path;
struct Config {
settings: HashMap<String, String>,
}
impl Config {
fn new() -> Self {
Config { settings: HashMap::new() }
}
fn load_from_file<P: AsRef<Path>>(&mut self, path: P) -> io::Result<()> {
let file = File::open(path)?;
for line in io::BufReader::new(file).lines() {
let line = line?;
if let Some((key, value)) = line.split_once('=') {
self.settings.insert(key.trim().to_string(), value.trim().to_string());
}
}
Ok(())
}
}
```
<edit>
<path>src/config.rs</path>
<operation>append_child</operation>
<symbol>#imports</symbol>
<description>Import necessary modules for file operations</description>
</edit>
<edit>
<path>src/config.rs</path>
<operation>append_child</operation>
<symbol>impl Config</symbol>
<description>Add load_from_file method</description>
</edit>
</step>
</message>
</example>
<example language="rust">
<message role="user">
```rs src/database.rs
pub(crate) struct Database {
connection: Connection,
}
impl Database {
fn new(url: &str) -> Result<Self, Error> {
let connection = Connection::connect(url)?;
Ok(Database { connection })
}
async fn query(&self, sql: &str) -> Result<Vec<Row>, Error> {
self.connection.query(sql, &[])
}
}
```
I want to add error handling to this module.
</message>
<message role="assistant">
<step>
Add error handling to the 'query' method and create a custom error type
```rust
use std::fmt;
use std::error::Error as StdError;
#[derive(Debug)]
pub enum DatabaseError {
ConnectionError(String),
QueryError(String),
}
impl fmt::Display for DatabaseError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
DatabaseError::ConnectionError(msg) => write!(f, "Connection error: {}", msg),
DatabaseError::QueryError(msg) => write!(f, "Query error: {}", msg),
}
}
}
impl StdError for DatabaseError {}
// ...omitted code...
impl Database {
fn new(url: &str) -> Result<Self, DatabaseError> {
let connection = Connection::connect(url)
.map_err(|e| DatabaseError::ConnectionError(e.to_string()))?;
Ok(Database { connection })
}
async fn query(&self, sql: &str) -> Result<Vec<Row>, DatabaseError> {
self.connection.query(sql, &[])
.await
.map_err(|e| DatabaseError::QueryError(e.to_string()))
}
}
```
<edit>
<path>src/database.rs</path>
<operation>prepend_child</operation>
<symbol>#imports</symbol>
<description>Import necessary error handling modules</description>
</edit>
<edit>
<path>src/database.rs</path>
<operation>insert_sibling_before</operation>
<symbol>pub(crate) struct Database</symbol>
<description>Define custom DatabaseError enum</description>
</edit>
<edit>
<path>src/database.rs</path>
<operation>update</operation>
<symbol>impl Database fn new</symbol>
<description>Update new method to use DatabaseError</description>
</edit>
<edit>
<path>src/database.rs</path>
<operation>update</operation>
<symbol>impl Database async fn query</symbol>
<description>Update query method to use DatabaseError</description>
</edit>
</step>
</message>
</example> </example>
You should think step by step. When possible, produce smaller, coherent logical steps as opposed to one big step that combines lots of heterogeneous edits. You should think step by step. When possible, produce smaller, coherent logical steps as opposed to one big step that combines lots of heterogeneous edits.

12
crates/assistant/src/context.rs
View file

@ -472,7 +472,7 @@ pub enum XmlTagKind {
Step, Step,
Edit, Edit,
Path, Path,
Symbol, Search,
Within, Within,
Operation, Operation,
Description, Description,
@ -1518,7 +1518,7 @@ impl Context {
if tag.kind == XmlTagKind::Edit && tag.is_open_tag { if tag.kind == XmlTagKind::Edit && tag.is_open_tag {
let mut path = None; let mut path = None;
let mut symbol = None; let mut search = None;
let mut operation = None; let mut operation = None;
let mut description = None; let mut description = None;
@ -1527,7 +1527,7 @@ impl Context {
edits.push(WorkflowStepEdit::new( edits.push(WorkflowStepEdit::new(
path, path,
operation, operation,
symbol, search,
description, description,
)); ));
break; break;
@ -1536,7 +1536,7 @@ impl Context {
if tag.is_open_tag if tag.is_open_tag
&& [ && [
XmlTagKind::Path, XmlTagKind::Path,
XmlTagKind::Symbol, XmlTagKind::Search,
XmlTagKind::Operation, XmlTagKind::Operation,
XmlTagKind::Description, XmlTagKind::Description,
] ]
@ -1555,8 +1555,8 @@ impl Context {
match kind { match kind {
XmlTagKind::Path => path = Some(content), XmlTagKind::Path => path = Some(content),
XmlTagKind::Operation => operation = Some(content), XmlTagKind::Operation => operation = Some(content),
XmlTagKind::Symbol => { XmlTagKind::Search => {
symbol = Some(content).filter(|s| !s.is_empty()) search = Some(content).filter(|s| !s.is_empty())
} }
XmlTagKind::Description => { XmlTagKind::Description => {
description = description =

44
crates/assistant/src/context/context_tests.rs
View file

@ -609,8 +609,8 @@ async fn test_workflow_step_parsing(cx: &mut TestAppContext) {
<edit>« <edit>«
<path>src/lib.rs</path> <path>src/lib.rs</path>
<operation>insert_sibling_after</operation> <operation>insert_after</operation>
<symbol>fn one</symbol> <search>fn one</search>
<description>add a `two` function</description> <description>add a `two` function</description>
</edit> </edit>
</step> </step>
@ -634,8 +634,8 @@ async fn test_workflow_step_parsing(cx: &mut TestAppContext) {
<edit> <edit>
<path>src/lib.rs</path> <path>src/lib.rs</path>
<operation>insert_sibling_after</operation> <operation>insert_after</operation>
<symbol>fn one</symbol> <search>fn one</search>
<description>add a `two` function</description> <description>add a `two` function</description>
</edit> </edit>
</step>» </step>»
@ -643,8 +643,8 @@ async fn test_workflow_step_parsing(cx: &mut TestAppContext) {
also,", also,",
&[&[WorkflowStepEdit { &[&[WorkflowStepEdit {
path: "src/lib.rs".into(), path: "src/lib.rs".into(),
kind: WorkflowStepEditKind::InsertSiblingAfter { kind: WorkflowStepEditKind::InsertAfter {
symbol: "fn one".into(), search: "fn one".into(),
description: "add a `two` function".into(), description: "add a `two` function".into(),
}, },
}]], }]],
@ -668,8 +668,8 @@ async fn test_workflow_step_parsing(cx: &mut TestAppContext) {
<edit> <edit>
<path>src/lib.rs</path> <path>src/lib.rs</path>
<operation>insert_sibling_after</operation> <operation>insert_after</operation>
<symbol>«fn zero»</symbol> <search>«fn zero»</search>
<description>add a `two` function</description> <description>add a `two` function</description>
</edit> </edit>
</step> </step>
@ -693,8 +693,8 @@ async fn test_workflow_step_parsing(cx: &mut TestAppContext) {
<edit> <edit>
<path>src/lib.rs</path> <path>src/lib.rs</path>
<operation>insert_sibling_after</operation> <operation>insert_after</operation>
<symbol>fn zero</symbol> <search>fn zero</search>
<description>add a `two` function</description> <description>add a `two` function</description>
</edit> </edit>
</step>» </step>»
@ -702,8 +702,8 @@ async fn test_workflow_step_parsing(cx: &mut TestAppContext) {
also,", also,",
&[&[WorkflowStepEdit { &[&[WorkflowStepEdit {
path: "src/lib.rs".into(), path: "src/lib.rs".into(),
kind: WorkflowStepEditKind::InsertSiblingAfter { kind: WorkflowStepEditKind::InsertAfter {
symbol: "fn zero".into(), search: "fn zero".into(),
description: "add a `two` function".into(), description: "add a `two` function".into(),
}, },
}]], }]],
@ -731,8 +731,8 @@ async fn test_workflow_step_parsing(cx: &mut TestAppContext) {
<edit> <edit>
<path>src/lib.rs</path> <path>src/lib.rs</path>
<operation>insert_sibling_after</operation> <operation>insert_after</operation>
<symbol>fn zero</symbol> <search>fn zero</search>
<description>add a `two` function</description> <description>add a `two` function</description>
</edit> </edit>
</step> </step>
@ -762,8 +762,8 @@ async fn test_workflow_step_parsing(cx: &mut TestAppContext) {
<edit> <edit>
<path>src/lib.rs</path> <path>src/lib.rs</path>
<operation>insert_sibling_after</operation> <operation>insert_after</operation>
<symbol>fn zero</symbol> <search>fn zero</search>
<description>add a `two` function</description> <description>add a `two` function</description>
</edit> </edit>
</step>» </step>»
@ -771,8 +771,8 @@ async fn test_workflow_step_parsing(cx: &mut TestAppContext) {
also,", also,",
&[&[WorkflowStepEdit { &[&[WorkflowStepEdit {
path: "src/lib.rs".into(), path: "src/lib.rs".into(),
kind: WorkflowStepEditKind::InsertSiblingAfter { kind: WorkflowStepEditKind::InsertAfter {
symbol: "fn zero".into(), search: "fn zero".into(),
description: "add a `two` function".into(), description: "add a `two` function".into(),
}, },
}]], }]],
@ -808,8 +808,8 @@ async fn test_workflow_step_parsing(cx: &mut TestAppContext) {
<edit> <edit>
<path>src/lib.rs</path> <path>src/lib.rs</path>
<operation>insert_sibling_after</operation> <operation>insert_after</operation>
<symbol>fn zero</symbol> <search>fn zero</search>
<description>add a `two` function</description> <description>add a `two` function</description>
</edit> </edit>
</step>» </step>»
@ -817,8 +817,8 @@ async fn test_workflow_step_parsing(cx: &mut TestAppContext) {
also,", also,",
&[&[WorkflowStepEdit { &[&[WorkflowStepEdit {
path: "src/lib.rs".into(), path: "src/lib.rs".into(),
kind: WorkflowStepEditKind::InsertSiblingAfter { kind: WorkflowStepEditKind::InsertAfter {
symbol: "fn zero".into(), search: "fn zero".into(),
description: "add a `two` function".into(), description: "add a `two` function".into(),
}, },
}]], }]],

524
crates/assistant/src/workflow.rs
View file

@ -4,16 +4,14 @@ use collections::HashMap;
use editor::Editor; use editor::Editor;
use gpui::AsyncAppContext; use gpui::AsyncAppContext;
use gpui::{Model, Task, UpdateGlobal as _, View, WeakView, WindowContext}; use gpui::{Model, Task, UpdateGlobal as _, View, WeakView, WindowContext};
use language::{Anchor, Buffer, BufferSnapshot, Outline, OutlineItem, ParseStatus, SymbolPath}; use language::{Buffer, BufferSnapshot};
use project::{Project, ProjectPath}; use project::{Project, ProjectPath};
use rope::Point;
use schemars::JsonSchema; use schemars::JsonSchema;
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
use std::{ops::Range, path::Path, sync::Arc}; use std::{ops::Range, path::Path, sync::Arc};
use text::Bias;
use workspace::Workspace; use workspace::Workspace;
const IMPORTS_SYMBOL: &str = "#imports";
#[derive(Debug)] #[derive(Debug)]
pub(crate) struct WorkflowStep { pub(crate) struct WorkflowStep {
pub range: Range<language::Anchor>, pub range: Range<language::Anchor>,
@ -45,35 +43,21 @@ pub struct WorkflowSuggestionGroup {
#[derive(Clone, Debug, Eq, PartialEq)] #[derive(Clone, Debug, Eq, PartialEq)]
pub enum WorkflowSuggestion { pub enum WorkflowSuggestion {
Update { Update {
symbol_path: SymbolPath,
range: Range<language::Anchor>, range: Range<language::Anchor>,
description: String, description: String,
}, },
CreateFile { CreateFile {
description: String, description: String,
}, },
InsertSiblingBefore { InsertBefore {
symbol_path: SymbolPath,
position: language::Anchor, position: language::Anchor,
description: String, description: String,
}, },
InsertSiblingAfter { InsertAfter {
symbol_path: SymbolPath,
position: language::Anchor,
description: String,
},
PrependChild {
symbol_path: Option<SymbolPath>,
position: language::Anchor,
description: String,
},
AppendChild {
symbol_path: Option<SymbolPath>,
position: language::Anchor, position: language::Anchor,
description: String, description: String,
}, },
Delete { Delete {
symbol_path: SymbolPath,
range: Range<language::Anchor>, range: Range<language::Anchor>,
}, },
} }
@ -83,10 +67,9 @@ impl WorkflowSuggestion {
match self { match self {
Self::Update { range, .. } => range.clone(), Self::Update { range, .. } => range.clone(),
Self::CreateFile { .. } => language::Anchor::MIN..language::Anchor::MAX, Self::CreateFile { .. } => language::Anchor::MIN..language::Anchor::MAX,
Self::InsertSiblingBefore { position, .. } Self::InsertBefore { position, .. } | Self::InsertAfter { position, .. } => {
| Self::InsertSiblingAfter { position, .. } *position..*position
| Self::PrependChild { position, .. } }
| Self::AppendChild { position, .. } => *position..*position,
Self::Delete { range, .. } => range.clone(), Self::Delete { range, .. } => range.clone(),
} }
} }
@ -95,10 +78,8 @@ impl WorkflowSuggestion {
match self { match self {
Self::Update { description, .. } Self::Update { description, .. }
| Self::CreateFile { description } | Self::CreateFile { description }
| Self::InsertSiblingBefore { description, .. } | Self::InsertBefore { description, .. }
| Self::InsertSiblingAfter { description, .. } | Self::InsertAfter { description, .. } => Some(description),
| Self::PrependChild { description, .. }
| Self::AppendChild { description, .. } => Some(description),
Self::Delete { .. } => None, Self::Delete { .. } => None,
} }
} }
@ -107,10 +88,8 @@ impl WorkflowSuggestion {
match self { match self {
Self::Update { description, .. } Self::Update { description, .. }
| Self::CreateFile { description } | Self::CreateFile { description }
| Self::InsertSiblingBefore { description, .. } | Self::InsertBefore { description, .. }
| Self::InsertSiblingAfter { description, .. } | Self::InsertAfter { description, .. } => Some(description),
| Self::PrependChild { description, .. }
| Self::AppendChild { description, .. } => Some(description),
Self::Delete { .. } => None, Self::Delete { .. } => None,
} }
} }
@ -161,7 +140,7 @@ impl WorkflowSuggestion {
initial_prompt = description.clone(); initial_prompt = description.clone();
suggestion_range = editor::Anchor::min()..editor::Anchor::min(); suggestion_range = editor::Anchor::min()..editor::Anchor::min();
} }
Self::InsertSiblingBefore { Self::InsertBefore {
position, position,
description, description,
.. ..
@ -178,7 +157,7 @@ impl WorkflowSuggestion {
line_start..line_start line_start..line_start
}); });
} }
Self::InsertSiblingAfter { Self::InsertAfter {
position, position,
description, description,
.. ..
@ -195,40 +174,6 @@ impl WorkflowSuggestion {
line_start..line_start line_start..line_start
}); });
} }
Self::PrependChild {
position,
description,
..
} => {
let position = snapshot.anchor_in_excerpt(excerpt_id, *position)?;
initial_prompt = description.clone();
suggestion_range = buffer.update(cx, |buffer, cx| {
buffer.start_transaction(cx);
let line_start = buffer.insert_empty_line(position, false, true, cx);
initial_transaction_id = buffer.end_transaction(cx);
buffer.refresh_preview(cx);
let line_start = buffer.read(cx).anchor_before(line_start);
line_start..line_start
});
}
Self::AppendChild {
position,
description,
..
} => {
let position = snapshot.anchor_in_excerpt(excerpt_id, *position)?;
initial_prompt = description.clone();
suggestion_range = buffer.update(cx, |buffer, cx| {
buffer.start_transaction(cx);
let line_start = buffer.insert_empty_line(position, true, false, cx);
initial_transaction_id = buffer.end_transaction(cx);
buffer.refresh_preview(cx);
let line_start = buffer.read(cx).anchor_before(line_start);
line_start..line_start
});
}
Self::Delete { range, .. } => { Self::Delete { range, .. } => {
initial_prompt = "Delete".to_string(); initial_prompt = "Delete".to_string();
suggestion_range = snapshot.anchor_in_excerpt(excerpt_id, range.start)? suggestion_range = snapshot.anchor_in_excerpt(excerpt_id, range.start)?
@ -254,7 +199,7 @@ impl WorkflowStepEdit {
pub fn new( pub fn new(
path: Option<String>, path: Option<String>,
operation: Option<String>, operation: Option<String>,
symbol: Option<String>, search: Option<String>,
description: Option<String>, description: Option<String>,
) -> Result<Self> { ) -> Result<Self> {
let path = path.ok_or_else(|| anyhow!("missing path"))?; let path = path.ok_or_else(|| anyhow!("missing path"))?;
@ -262,27 +207,19 @@ impl WorkflowStepEdit {
let kind = match operation.as_str() { let kind = match operation.as_str() {
"update" => WorkflowStepEditKind::Update { "update" => WorkflowStepEditKind::Update {
symbol: symbol.ok_or_else(|| anyhow!("missing symbol"))?, search: search.ok_or_else(|| anyhow!("missing search"))?,
description: description.ok_or_else(|| anyhow!("missing description"))?, description: description.ok_or_else(|| anyhow!("missing description"))?,
}, },
"insert_sibling_before" => WorkflowStepEditKind::InsertSiblingBefore { "insert_before" => WorkflowStepEditKind::InsertBefore {
symbol: symbol.ok_or_else(|| anyhow!("missing symbol"))?, search: search.ok_or_else(|| anyhow!("missing search"))?,
description: description.ok_or_else(|| anyhow!("missing description"))?, description: description.ok_or_else(|| anyhow!("missing description"))?,
}, },
"insert_sibling_after" => WorkflowStepEditKind::InsertSiblingAfter { "insert_after" => WorkflowStepEditKind::InsertAfter {
symbol: symbol.ok_or_else(|| anyhow!("missing symbol"))?, search: search.ok_or_else(|| anyhow!("missing search"))?,
description: description.ok_or_else(|| anyhow!("missing description"))?,
},
"prepend_child" => WorkflowStepEditKind::PrependChild {
symbol,
description: description.ok_or_else(|| anyhow!("missing description"))?,
},
"append_child" => WorkflowStepEditKind::AppendChild {
symbol,
description: description.ok_or_else(|| anyhow!("missing description"))?, description: description.ok_or_else(|| anyhow!("missing description"))?,
}, },
"delete" => WorkflowStepEditKind::Delete { "delete" => WorkflowStepEditKind::Delete {
symbol: symbol.ok_or_else(|| anyhow!("missing symbol"))?, search: search.ok_or_else(|| anyhow!("missing search"))?,
}, },
"create" => WorkflowStepEditKind::Create { "create" => WorkflowStepEditKind::Create {
description: description.ok_or_else(|| anyhow!("missing description"))?, description: description.ok_or_else(|| anyhow!("missing description"))?,
@ -323,200 +260,143 @@ impl WorkflowStepEdit {
})?? })??
.await?; .await?;
let mut parse_status = buffer.read_with(&cx, |buffer, _cx| buffer.parse_status())?;
while *parse_status.borrow() != ParseStatus::Idle {
parse_status.changed().await?;
}
let snapshot = buffer.update(&mut cx, |buffer, _| buffer.snapshot())?; let snapshot = buffer.update(&mut cx, |buffer, _| buffer.snapshot())?;
let outline = snapshot.outline(None).context("no outline for buffer")?; let suggestion = cx
.background_executor()
let suggestion = match kind { .spawn(async move {
WorkflowStepEditKind::Update { match kind {
symbol, WorkflowStepEditKind::Update {
description, search,
} => {
let (symbol_path, symbol) = Self::resolve_symbol(&snapshot, &outline, &symbol)?;
let start = symbol
.annotation_range
.map_or(symbol.range.start, |range| range.start);
let start = Point::new(start.row, 0);
let end = Point::new(
symbol.range.end.row,
snapshot.line_len(symbol.range.end.row),
);
let range = snapshot.anchor_before(start)..snapshot.anchor_after(end);
WorkflowSuggestion::Update {
range,
description,
symbol_path,
}
}
WorkflowStepEditKind::Create { description } => {
WorkflowSuggestion::CreateFile { description }
}
WorkflowStepEditKind::InsertSiblingBefore {
symbol,
description,
} => {
let (symbol_path, symbol) = Self::resolve_symbol(&snapshot, &outline, &symbol)?;
let position = snapshot.anchor_before(
symbol
.annotation_range
.map_or(symbol.range.start, |annotation_range| {
annotation_range.start
}),
);
WorkflowSuggestion::InsertSiblingBefore {
position,
description,
symbol_path,
}
}
WorkflowStepEditKind::InsertSiblingAfter {
symbol,
description,
} => {
let (symbol_path, symbol) = Self::resolve_symbol(&snapshot, &outline, &symbol)?;
let position = snapshot.anchor_after(symbol.range.end);
WorkflowSuggestion::InsertSiblingAfter {
position,
description,
symbol_path,
}
}
WorkflowStepEditKind::PrependChild {
symbol,
description,
} => {
if let Some(symbol) = symbol {
let (symbol_path, symbol) = Self::resolve_symbol(&snapshot, &outline, &symbol)?;
let position = snapshot.anchor_after(
symbol
.body_range
.map_or(symbol.range.start, |body_range| body_range.start),
);
WorkflowSuggestion::PrependChild {
position,
description, description,
symbol_path: Some(symbol_path), } => {
let range = Self::resolve_location(&snapshot, &search);
WorkflowSuggestion::Update { range, description }
} }
} else { WorkflowStepEditKind::Create { description } => {
WorkflowSuggestion::PrependChild { WorkflowSuggestion::CreateFile { description }
position: language::Anchor::MIN, }
WorkflowStepEditKind::InsertBefore {
search,
description, description,
symbol_path: None, } => {
let range = Self::resolve_location(&snapshot, &search);
WorkflowSuggestion::InsertBefore {
position: range.start,
description,
}
}
WorkflowStepEditKind::InsertAfter {
search,
description,
} => {
let range = Self::resolve_location(&snapshot, &search);
WorkflowSuggestion::InsertAfter {
position: range.end,
description,
}
}
WorkflowStepEditKind::Delete { search } => {
let range = Self::resolve_location(&snapshot, &search);
WorkflowSuggestion::Delete { range }
} }
} }
} })
WorkflowStepEditKind::AppendChild { .await;
symbol,
description,
} => {
if let Some(symbol) = symbol {
let (symbol_path, symbol) = Self::resolve_symbol(&snapshot, &outline, &symbol)?;
let position = snapshot.anchor_before(
symbol
.body_range
.map_or(symbol.range.end, |body_range| body_range.end),
);
WorkflowSuggestion::AppendChild {
position,
description,
symbol_path: Some(symbol_path),
}
} else {
WorkflowSuggestion::PrependChild {
position: language::Anchor::MAX,
description,
symbol_path: None,
}
}
}
WorkflowStepEditKind::Delete { symbol } => {
let (symbol_path, symbol) = Self::resolve_symbol(&snapshot, &outline, &symbol)?;
let start = symbol
.annotation_range
.map_or(symbol.range.start, |range| range.start);
let start = Point::new(start.row, 0);
let end = Point::new(
symbol.range.end.row,
snapshot.line_len(symbol.range.end.row),
);
let range = snapshot.anchor_before(start)..snapshot.anchor_after(end);
WorkflowSuggestion::Delete { range, symbol_path }
}
};
Ok((buffer, suggestion)) Ok((buffer, suggestion))
} }
fn resolve_symbol( fn resolve_location(buffer: &text::BufferSnapshot, search_query: &str) -> Range<text::Anchor> {
snapshot: &BufferSnapshot, const INSERTION_SCORE: f64 = -1.0;
outline: &Outline<Anchor>, const DELETION_SCORE: f64 = -1.0;
symbol: &str, const REPLACEMENT_SCORE: f64 = -1.0;
) -> Result<(SymbolPath, OutlineItem<Point>)> { const EQUALITY_SCORE: f64 = 5.0;
if symbol == IMPORTS_SYMBOL {
let target_row = find_first_non_comment_line(snapshot); struct Matrix {
Ok(( cols: usize,
SymbolPath(IMPORTS_SYMBOL.to_string()), data: Vec<f64>,
OutlineItem {
range: Point::new(target_row, 0)..Point::new(target_row + 1, 0),
..Default::default()
},
))
} else {
let (symbol_path, symbol) = outline
.find_most_similar(symbol)
.with_context(|| format!("symbol not found: {symbol}"))?;
Ok((symbol_path, symbol.to_point(snapshot)))
} }
impl Matrix {
fn new(rows: usize, cols: usize) -> Self {
Matrix {
cols,
data: vec![0.0; rows * cols],
}
}
fn get(&self, row: usize, col: usize) -> f64 {
self.data[row * self.cols + col]
}
fn set(&mut self, row: usize, col: usize, value: f64) {
self.data[row * self.cols + col] = value;
}
}
let buffer_len = buffer.len();
let query_len = search_query.len();
let mut matrix = Matrix::new(query_len + 1, buffer_len + 1);
for (i, query_byte) in search_query.bytes().enumerate() {
for (j, buffer_byte) in buffer.bytes_in_range(0..buffer.len()).flatten().enumerate() {
let match_score = if query_byte == *buffer_byte {
EQUALITY_SCORE
} else {
REPLACEMENT_SCORE
};
let up = matrix.get(i + 1, j) + DELETION_SCORE;
let left = matrix.get(i, j + 1) + INSERTION_SCORE;
let diagonal = matrix.get(i, j) + match_score;
let score = up.max(left.max(diagonal)).max(0.);
matrix.set(i + 1, j + 1, score);
}
}
// Traceback to find the best match
let mut best_buffer_end = buffer_len;
let mut best_score = 0.0;
for col in 1..=buffer_len {
let score = matrix.get(query_len, col);
if score > best_score {
best_score = score;
best_buffer_end = col;
}
}
let mut query_ix = query_len;
let mut buffer_ix = best_buffer_end;
while query_ix > 0 && buffer_ix > 0 {
let current = matrix.get(query_ix, buffer_ix);
let up = matrix.get(query_ix - 1, buffer_ix);
let left = matrix.get(query_ix, buffer_ix - 1);
if current == left + INSERTION_SCORE {
buffer_ix -= 1;
} else if current == up + DELETION_SCORE {
query_ix -= 1;
} else {
query_ix -= 1;
buffer_ix -= 1;
}
}
let mut start = buffer.offset_to_point(buffer.clip_offset(buffer_ix, Bias::Left));
start.column = 0;
let mut end = buffer.offset_to_point(buffer.clip_offset(best_buffer_end, Bias::Right));
end.column = buffer.line_len(end.row);
buffer.anchor_after(start)..buffer.anchor_before(end)
} }
} }
fn find_first_non_comment_line(snapshot: &BufferSnapshot) -> u32 {
let Some(language) = snapshot.language() else {
return 0;
};
let scope = language.default_scope();
let comment_prefixes = scope.line_comment_prefixes();
let mut chunks = snapshot.as_rope().chunks();
let mut target_row = 0;
loop {
let starts_with_comment = chunks
.peek()
.map(|chunk| {
comment_prefixes
.iter()
.any(|s| chunk.starts_with(s.as_ref().trim_end()))
})
.unwrap_or(false);
if !starts_with_comment {
break;
}
target_row += 1;
if !chunks.next_line() {
break;
}
}
target_row
}
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize, JsonSchema)]
#[serde(tag = "operation")] #[serde(tag = "operation")]
pub enum WorkflowStepEditKind { pub enum WorkflowStepEditKind {
/// Rewrites the specified symbol entirely based on the given description. /// Rewrites the specified text entirely based on the given description.
/// This operation completely replaces the existing symbol with new content. /// This operation completely replaces the given text.
Update { Update {
/// A fully-qualified reference to the symbol, e.g. `mod foo impl Bar pub fn baz` instead of just `fn baz`. /// A string in the source text to apply the update to.
/// The path should uniquely identify the symbol within the containing file. search: String,
symbol: String,
/// A brief description of the transformation to apply to the symbol. /// A brief description of the transformation to apply to the symbol.
description: String, description: String,
}, },
@ -526,47 +406,101 @@ pub enum WorkflowStepEditKind {
/// A brief description of the file to be created. /// A brief description of the file to be created.
description: String, description: String,
}, },
/// Inserts a new symbol based on the given description before the specified symbol. /// Inserts text before the specified text in the source file.
/// This operation adds new content immediately preceding an existing symbol. InsertBefore {
InsertSiblingBefore { /// A string in the source text to insert text before.
/// A fully-qualified reference to the symbol, e.g. `mod foo impl Bar pub fn baz` instead of just `fn baz`. search: String,
/// The new content will be inserted immediately before this symbol. /// A brief description of how the new text should be generated.
symbol: String,
/// A brief description of the new symbol to be inserted.
description: String, description: String,
}, },
/// Inserts a new symbol based on the given description after the specified symbol. /// Inserts text after the specified text in the source file.
/// This operation adds new content immediately following an existing symbol. InsertAfter {
InsertSiblingAfter { /// A string in the source text to insert text after.
/// A fully-qualified reference to the symbol, e.g. `mod foo impl Bar pub fn baz` instead of just `fn baz`. search: String,
/// The new content will be inserted immediately after this symbol. /// A brief description of how the new text should be generated.
symbol: String,
/// A brief description of the new symbol to be inserted.
description: String,
},
/// Inserts a new symbol as a child of the specified symbol at the start.
/// This operation adds new content as the first child of an existing symbol (or file if no symbol is provided).
PrependChild {
/// An optional fully-qualified reference to the symbol after the code you want to insert, e.g. `mod foo impl Bar pub fn baz` instead of just `fn baz`.
/// If provided, the new content will be inserted as the first child of this symbol.
/// If not provided, the new content will be inserted at the top of the file.
symbol: Option<String>,
/// A brief description of the new symbol to be inserted.
description: String,
},
/// Inserts a new symbol as a child of the specified symbol at the end.
/// This operation adds new content as the last child of an existing symbol (or file if no symbol is provided).
AppendChild {
/// An optional fully-qualified reference to the symbol before the code you want to insert, e.g. `mod foo impl Bar pub fn baz` instead of just `fn baz`.
/// If provided, the new content will be inserted as the last child of this symbol.
/// If not provided, the new content will be applied at the bottom of the file.
symbol: Option<String>,
/// A brief description of the new symbol to be inserted.
description: String, description: String,
}, },
/// Deletes the specified symbol from the containing file. /// Deletes the specified symbol from the containing file.
Delete { Delete {
/// An fully-qualified reference to the symbol to be deleted, e.g. `mod foo impl Bar pub fn baz` instead of just `fn baz`. /// A string in the source text to delete.
symbol: String, search: String,
}, },
} }
#[cfg(test)]
mod tests {
use super::*;
use gpui::{AppContext, Context};
use text::{OffsetRangeExt, Point};
#[gpui::test]
fn test_resolve_location(cx: &mut AppContext) {
{
let buffer = cx.new_model(|cx| {
Buffer::local(
concat!(
" Lorem\n",
" ipsum\n",
" dolor sit amet\n",
" consecteur",
),
cx,
)
});
let snapshot = buffer.read(cx).snapshot();
assert_eq!(
WorkflowStepEdit::resolve_location(&snapshot, "ipsum\ndolor").to_point(&snapshot),
Point::new(1, 0)..Point::new(2, 18)
);
}
{
let buffer = cx.new_model(|cx| {
Buffer::local(
concat!(
"fn foo1(a: usize) -> usize {\n",
" 42\n",
"}\n",
"\n",
"fn foo2(b: usize) -> usize {\n",
" 42\n",
"}\n",
),
cx,
)
});
let snapshot = buffer.read(cx).snapshot();
assert_eq!(
WorkflowStepEdit::resolve_location(&snapshot, "fn foo1(b: usize) {\n42\n}")
.to_point(&snapshot),
Point::new(0, 0)..Point::new(2, 1)
);
}
{
let buffer = cx.new_model(|cx| {
Buffer::local(
concat!(
"fn main() {\n",
" Foo\n",
" .bar()\n",
" .baz()\n",
" .qux()\n",
"}\n",
"\n",
"fn foo2(b: usize) -> usize {\n",
" 42\n",
"}\n",
),
cx,
)
});
let snapshot = buffer.read(cx).snapshot();
assert_eq!(
WorkflowStepEdit::resolve_location(&snapshot, "Foo.bar.baz.qux()")
.to_point(&snapshot),
Point::new(1, 0)..Point::new(4, 14)
);
}
}
}