Skip to content

Commit

Permalink
- Fixes for two potential server crashes
Browse files Browse the repository at this point in the history
- Fix for server git repo remaining in locked state after a crash, which caused various issues
- Fix for server git user and email not being set in some environments (#8)
- Fix for 'replacements failed' error that was popping up in some circumstances
- Fix for build issue that could cause large updates to fail, take too long, or use too many tokens in some circumstances
- Clean up extraneous logging
- Prompt update to prevent ouputting files at absolute paths (like '/etc/config.txt')
- Prompt update to prevent sometimes using file block format for explanations, causing explanations to be outputted as files
- Prompt update to prevent stopping before the the plan is really finished
- Increase maximum number of auto-continuations to 50 (from 30)
  • Loading branch information
danenania committed Apr 2, 2024
1 parent 45236b3 commit c16980c
Show file tree
Hide file tree
Showing 15 changed files with 111 additions and 45 deletions.
34 changes: 34 additions & 0 deletions app/server/db/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ import (
"bytes"
"fmt"
"log"
"os"
"os/exec"
"path/filepath"
"strconv"
"strings"
"time"
Expand All @@ -29,6 +31,14 @@ func InitGitRepo(orgId, planId string) error {
return fmt.Errorf("error initializing git repository with 'main' as default branch for dir: %s, err: %v, output: %s", dir, err, string(res))
}

// Configure user name and email for the repository
if err := setGitConfig(dir, "user.email", "[email protected]"); err != nil {
return err
}
if err := setGitConfig(dir, "user.name", "Plandex"); err != nil {
return err
}

return nil
}

Expand Down Expand Up @@ -314,3 +324,27 @@ func gitCommit(repoDir, commitMsg string) error {

return nil
}

func gitRemoveIndexLockFileIfExists(repoDir string) error {
// Remove the lock file if it exists
lockFilePath := filepath.Join(repoDir, ".git", "index.lock")
_, err := os.Stat(lockFilePath)

if err == nil {
if err := os.Remove(lockFilePath); err != nil {
return fmt.Errorf("error removing lock file: %v", err)
}
} else if !os.IsNotExist(err) {
return fmt.Errorf("error checking lock file: %v", err)
}

return nil
}

func setGitConfig(repoDir, key, value string) error {
res, err := exec.Command("git", "-C", repoDir, "config", key, value).CombinedOutput()
if err != nil {
return fmt.Errorf("error setting git config %s to %s for dir: %s, err: %v, output: %s", key, value, repoDir, err, string(res))
}
return nil
}
7 changes: 7 additions & 0 deletions app/server/db/locks.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,13 @@ func lockRepo(params LockRepoParams, numRetry int) (string, error) {
return "", fmt.Errorf("error inserting new lock: %v", err)
}

// check if git lock file exists
// remove it if so
err = gitRemoveIndexLockFileIfExists(getPlanDir(orgId, planId))
if err != nil {
return "", fmt.Errorf("error removing lock file: %v", err)
}

branches, err := GitListBranches(orgId, planId)
if err != nil {
return "", fmt.Errorf("error getting branches: %v", err)
Expand Down
3 changes: 2 additions & 1 deletion app/server/db/org_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,8 @@ func AddOrgDomainUsers(orgId, domain string, tx *sql.Tx) error {
var valueStrings []string
var valueArgs []interface{}
for i, user := range usersForDomain {
valueStrings = append(valueStrings, fmt.Sprintf("($%d, $%d)", i*2+1, i*2+2))
num := i * 2
valueStrings = append(valueStrings, fmt.Sprintf("($%d, $%d)", num+1, num+2))
valueArgs = append(valueArgs, orgId, user.Id)
}

Expand Down
21 changes: 10 additions & 11 deletions app/server/handlers/settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"reflect"
"strings"

"github.com/davecgh/go-spew/spew"
"github.com/gorilla/mux"
"github.com/plandex/plandex/shared"
)
Expand Down Expand Up @@ -113,11 +112,11 @@ func UpdateSettingsHandler(w http.ResponseWriter, r *http.Request) {
return
}

log.Println("Original settings:")
spew.Dump(originalSettings)
// log.Println("Original settings:")
// spew.Dump(originalSettings)

log.Println("req.Settings:")
spew.Dump(req.Settings)
// log.Println("req.Settings:")
// spew.Dump(req.Settings)

err = db.StorePlanSettings(plan, req.Settings)

Expand Down Expand Up @@ -155,7 +154,7 @@ func UpdateSettingsHandler(w http.ResponseWriter, r *http.Request) {
}

func getUpdateCommitMsg(settings *shared.PlanSettings, originalSettings *shared.PlanSettings) string {
log.Println("Comparing settings")
// log.Println("Comparing settings")
// log.Println("Original:")
// spew.Dump(originalSettings)
// log.Println("New:")
Expand All @@ -168,7 +167,7 @@ func getUpdateCommitMsg(settings *shared.PlanSettings, originalSettings *shared.
return "No changes to settings"
}

log.Println("Changes to settings:", strings.Join(changes, "\n"))
// log.Println("Changes to settings:", strings.Join(changes, "\n"))

s := "⚙️ Updated model settings:"

Expand Down Expand Up @@ -203,9 +202,9 @@ func compareAny(a, b interface{}, path string, changes *[]string) {
bVal = bVal.Elem()
}

log.Println("Comparing", path, aVal.Kind(), bVal.Kind())
log.Println("aVal", aVal)
log.Println("bVal", bVal)
// log.Println("Comparing", path, aVal.Kind(), bVal.Kind())
// log.Println("aVal", aVal)
// log.Println("bVal", bVal)

switch aVal.Kind() {
case reflect.Struct:
Expand All @@ -230,7 +229,7 @@ func compareAny(a, b interface{}, path string, changes *[]string) {
}
}

log.Println("field", fieldName, "updatedPath", updatedPath)
// log.Println("field", fieldName, "updatedPath", updatedPath)

compareAny(aVal.Field(i).Interface(), bVal.Field(i).Interface(),
updatedPath, changes)
Expand Down
6 changes: 3 additions & 3 deletions app/server/model/plan/build_stream.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ func (fileState *activeBuildStreamFileState) listenStream(stream *openai.ChatCom

if err == nil {
log.Printf("File %s: Parsed streamed replacements\n", filePath)
spew.Dump(streamed)
// spew.Dump(streamed)

planFileResult, allSucceeded := getPlanResult(
planResultParams{
Expand Down Expand Up @@ -187,8 +187,8 @@ func (fileState *activeBuildStreamFileState) listenStream(stream *openai.ChatCom
return
} else if len(delta.ToolCalls) == 0 {
log.Println("Stream chunk missing function call. Response:")
spew.Dump(response)
spew.Dump(fileState)
log.Println(spew.Sdump(response))
log.Println(spew.Sdump(fileState))

fileState.retryOrError(fmt.Errorf("stream chunk missing function call. Reason: %s, File: %s", choice.FinishReason, filePath))
return
Expand Down
4 changes: 2 additions & 2 deletions app/server/model/plan/exec_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func ExecStatusShouldContinue(client *openai.Client, config shared.TaskRoleConfi
messages := []openai.ChatCompletionMessage{
{
Role: openai.ChatMessageRoleSystem,
Content: prompts.GetExecStatusShouldContinue(prompt, message), // Ensure this function is correctly defined in your package
Content: prompts.GetExecStatusShouldContinue(prompt, message),
},
}

Expand Down Expand Up @@ -95,7 +95,7 @@ func ExecStatusShouldContinue(client *openai.Client, config shared.TaskRoleConfi

if strRes == "" {
log.Println("No shouldAutoContinue function call found in response")
spew.Dump(resp)
log.Println(spew.Sdump(resp))

// return false, fmt.Errorf("no shouldAutoContinue function call found in response")

Expand Down
3 changes: 1 addition & 2 deletions app/server/model/plan/tell_exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,9 +218,8 @@ func execTellPlan(
state.replyId = uuid.New().String()
state.replyParser = types.NewReplyParser()

var promptMessage *openai.ChatCompletionMessage
if missingFileResponse == "" {

var promptMessage *openai.ChatCompletionMessage
if req.IsUserContinue {
if len(state.messages) == 0 {
active.StreamDoneCh <- &shared.ApiError{
Expand Down
17 changes: 13 additions & 4 deletions app/server/model/plan/tell_stream.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,11 @@ import (
"strings"
"time"

"github.com/davecgh/go-spew/spew"
"github.com/plandex/plandex/shared"
"github.com/sashabaranov/go-openai"
)

const MaxAutoContinueIterations = 30
const MaxAutoContinueIterations = 50

type activeTellStreamState struct {
client *openai.Client
Expand Down Expand Up @@ -203,6 +202,7 @@ func (state *activeTellStreamState) listenStream(stream *openai.ChatCompletionSt
errCh := make(chan error, 2)

go func() {
log.Println("getting description")
log.Println("getting description for assistant message: ", assistantMsg.Id)

if len(replyFiles) == 0 {
Expand Down Expand Up @@ -239,13 +239,22 @@ func (state *activeTellStreamState) listenStream(stream *openai.ChatCompletionSt
}

log.Println("Description stored")
spew.Dump(description)
// spew.Dump(description)

errCh <- nil
}()

go func() {
shouldContinue, err = ExecStatusShouldContinue(client, settings.ModelSet.ExecStatus, promptMessage.Content, assistantMsg.Message, active.Ctx)
// One of the below is nil occasionally, causing crash
log.Println("Getting exec status")
var prompt string
if promptMessage == nil {
log.Println("Prompt message is nil")
} else {
prompt = promptMessage.Content
}

shouldContinue, err = ExecStatusShouldContinue(client, settings.ModelSet.ExecStatus, prompt, assistantMsg.Message, active.Ctx)
if err != nil {
state.onError(fmt.Errorf("failed to get exec status: %v", err), false, assistantMsg.Id, convoCommitMsg)
errCh <- err
Expand Down
2 changes: 1 addition & 1 deletion app/server/model/plan/tell_summary.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func (state *activeTellStreamState) summarizeMessagesIfNeeded() bool {
// spew.Dump(s)

log.Println("tokensUpToTimestamp:")
spew.Dump(tokensUpToTimestamp)
log.Println(spew.Sdump(tokensUpToTimestamp))

active.StreamDoneCh <- &shared.ApiError{
Type: shared.ApiErrorTypeOther,
Expand Down
8 changes: 6 additions & 2 deletions app/server/model/prompts/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ const SysCreate = Identity + ` A plan is a set of files with an attached context
If code is being included for explanatory purposes and is not meant to be applied to a specific file, you MUST NOT label the code block in the format described in 2a. Instead, output the code without a label.
Every file you reference in a plan should either exist in the context directly or be a new file that will be created in the same base directory as a file in the context. For example, if there is a file in context at path 'lib/term.go', you can create a new file at path 'lib/utils_test.go' but *not* at path 'src/lib/term.go'. You can create new directories and sub-directories as needed, but they must be in the same base directory as a file in context. Don't ask the user to create new files or directories--you must do that yourself.
Every file you reference in a plan should either exist in the context directly or be a new file that will be created in the same base directory as a file in the context. For example, if there is a file in context at path 'lib/term.go', you can create a new file at path 'lib/utils_test.go' but *not* at path 'src/lib/term.go'. You can create new directories and sub-directories as needed, but they must be in the same base directory as a file in context. You must *never* create files with absolute paths like '/etc/config.txt'. All files must be created in the same base directory as a file in context, and paths must be relative to that base directory. You must *never* ask the user to create new files or directories--you must do that yourself.
**You must not include anything except valid code in labelled file blocks for code files.** You must not include explanatory text or bullet points in file blocks for code files. Only code. Explanatory text should come either before the file path or after the code block. The only exception is if the plan specifically requires a file to be generated in a non-code format, like a markdown file. In that case, you can include the non-code content in the file block. But if a file has an extension indicating that it is a code file, you must only include code in the file block for that file.
Expand All @@ -51,6 +51,8 @@ const SysCreate = Identity + ` A plan is a set of files with an attached context
You MUST NEVER use a file block that only contains comments describing an update or describing the file. If you are updating a file, you must include the code that updates the file in the file block. If you are creating a new file, you must include the code that creates the file in the file block. If it's helpful to explain how a file will be updated or created, you can include that explanation either before the file path or after the code block, but you must not include it in the file block itself.
You MUST NOT use the labelled file block format followed by triple backticks for **any purpose** other than creating or updating a file in the plan. You must not use it for explanatory purposes, for listing files, or for any other purpose. If you need to label a section or a list of files, use a markdown section header instead like this: '## Files to update'.
If code is being removed from a file, the removal must be shown in a labelled file block according to your instructions. Use a comment within the file block to denote the removal like '// Plandex: removed the fooBar function' or '// Plandex: removed the loop'. Do NOT use any other formatting apart from a labelled file block to denote the removal.
If a change is related to code in an existing file in context, make the change as an update to the existing file. Do NOT create a new file for a change that applies to an existing file in context. For example, if there is an 'Page.tsx' file in the existing context and the user has asked you to update the structure of the page component, make the change in the existing 'Page.tsx' file. Do NOT create a new file like 'page.tsx' or 'NewPage.tsx' for the change. If the user has specifically asked you to apply a change to a new file, then you can create a new file. If there is no existing file that makes sense to apply a change to, then you can create a new file.
Expand All @@ -75,6 +77,8 @@ const SysCreate = Identity + ` A plan is a set of files with an attached context
**You MUST NEVER give up and say the task is too large or complex for you to do.** Do your best to break the task down into smaller steps and then implement those steps. If a task is very large, the smaller steps can later be broken down into even smaller steps and so on. You can use as many responses as needed to complete a large task. Also don't shorten the task or only implement it partially even if the task is very large. Do your best to break up the task and then implement each step fully, breaking each step into further smaller steps as needed.
**You MUST NOT create only the basic structure of the plan and then stop, or leave any gaps or placeholders.** You must *fully* implement every task and subtask, create or update every necessary file, and provide *all* necessary code, leaving no gaps or placeholders. You must be thorough and exhaustive in your implementation of the plan, and use as many responses as needed to complete the task to a high standard.
## Working on subtasks
If you working on a subtask, first describe the subtask and what your approach will be, then implement it with code blocks. Apart from when you are following instruction 2b above to create the intial subtasks, you must not list, describe, or explain the subtask you are working on without an accompanying implementation in one or more code blocks. Describing what needs to be done to complete a subtask *DOES NOT* count as completing the subtask. It must be fully implemented with code blocks.
Expand All @@ -100,7 +104,7 @@ const SysCreate = Identity + ` A plan is a set of files with an attached context
At the end of each response, you can suggest additional iterations to make the plan better. You can also ask the user to load more files into context or give you more information if it would help you make a better plan.
At the *very* end of your response, in a final, separate paragraph, you *must* decide whether the plan is completed and if not, whether it should be automatically continued.
- If all the subtasks in a plan have been completed you must explictly say "All tasks have been completed."
- If all the subtasks in a plan have been thoroughly completed to a high standard, you must explictly say "All tasks have been completed."
Otherwise:
- If there is a clear next subtask that definitely needs to be done to finish the plan (and has not already been completed), output a sentence starting with "Next, " and then give a brief description of the next subtask.
- If there is no clear next subtask, or the user needs to take some action before you can continue, explicitly say "The plan cannot be continued." Then finish with a brief description of what the user needs to do for the plan to proceed.
Expand Down
9 changes: 6 additions & 3 deletions app/server/model/prompts/exec_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,12 @@ Set 'shouldContinue' to true if the plan should automatically continue based on
You must always call 'shouldAutoContinue'. Don't call any other function.`

func GetExecStatusShouldContinue(userPrompt, message string) string {
return SysExecStatusShouldContinue +
"\n\n**Here is the user's prompt:**\n" + userPrompt + "\n\n" +
"\n\n**Here is the latest message of the plan from AI 1:**\n" + message
s := SysExecStatusShouldContinue
if userPrompt != "" {
s += "\n\n**Here is the user's prompt:**\n" + userPrompt
}
s += "\n\n**Here is the latest message of the plan from AI 1:**\n" + message
return s
}

var ShouldAutoContinueFn = openai.FunctionDefinition{
Expand Down
13 changes: 10 additions & 3 deletions app/server/types/active_plan_pending_builds.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"fmt"
"log"
"plandex-server/db"

"github.com/plandex/plandex/shared"
)

func (ap *ActivePlan) PendingBuildsByPath(orgId, userId string, convoMessagesArg []*db.ConvoMessage) (map[string][]*ActiveBuild, error) {
Expand Down Expand Up @@ -63,15 +65,20 @@ func (ap *ActivePlan) PendingBuildsByPath(orgId, userId string, convoMessagesArg
activeBuildsByPath[file] = []*ActiveBuild{}
}

fileContent := parserRes.FileContents[i]

numTokens, err := shared.GetNumTokens(fileContent)

if err != nil {
return nil, fmt.Errorf("error getting tokens for file '%s': %v", file, err)
log.Printf("Error getting num tokens for file content: %v\n", err)
return nil, fmt.Errorf("error getting num tokens for file content: %v", err)
}

activeBuildsByPath[file] = append(activeBuildsByPath[file], &ActiveBuild{
ReplyId: desc.ConvoMessageId,
Idx: i,
FileContent: parserRes.FileContents[i],
FileContentTokens: parserRes.NumTokensByFile[file],
FileContent: fileContent,
FileContentTokens: numTokens,
Path: file,
FileDescription: parserRes.FileDescriptions[i],
})
Expand Down
3 changes: 1 addition & 2 deletions app/shared/plan_result.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package shared

import (
"log"
"time"
)

Expand Down Expand Up @@ -87,7 +86,7 @@ func (p PlanFileResultsByPath) ConflictedPaths(filesByPath map[string]string) ma
noConflicts := true
for _, res := range planRes {

log.Println("res:", res.Id)
// log.Println("res:", res.Id)
if len(res.Replacements) == 0 {
continue
}
Expand Down
16 changes: 5 additions & 11 deletions app/shared/plan_result_replacements.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,11 @@ import (

func ApplyReplacements(content string, replacements []*Replacement, setFailed bool) (string, bool) {
updated := content
lastInsertedIdx := 0

allSucceeded := true

for _, replacement := range replacements {
pre := updated[:lastInsertedIdx]
sub := updated[lastInsertedIdx:]
originalIdx := strings.Index(sub, replacement.Old)
originalIdx := strings.Index(updated, replacement.Old)

// log.Println("originalIdx:", originalIdx)

Expand All @@ -28,17 +25,14 @@ func ApplyReplacements(content string, replacements []*Replacement, setFailed bo
replacement.Failed = true
}

log.Println("Replacement failed: ")
log.Println("Replacement failed:")
log.Println(spew.Sdump(replacement))

log.Println("Sub: ")
log.Println(spew.Sdump(sub))
log.Println("Updated:")
log.Println(updated)

} else {
replaced := strings.Replace(sub, replacement.Old, replacement.New, 1)

updated = pre + replaced
lastInsertedIdx = lastInsertedIdx + originalIdx + len(replacement.New)
updated = strings.Replace(updated, replacement.Old, replacement.New, 1)
}
}

Expand Down
10 changes: 10 additions & 0 deletions releases/server/versions/0.8.1.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
- Fixes for two potential server crashes
- Fix for server git repo remaining in locked state after a crash, which caused various issues
- Fix for server git user and email not being set in some environments (https://github.com/plandex-ai/plandex/issues/8)
- Fix for 'replacements failed' error that was popping up in some circumstances
- Fix for build issue that could cause large updates to fail, take too long, or use too many tokens in some circumstances
- Clean up extraneous logging
- Prompt update to prevent ouputting files at absolute paths (like '/etc/config.txt')
- Prompt update to prevent sometimes using file block format for explanations, causing explanations to be outputted as files
- Prompt update to prevent stopping before the the plan is really finished
- Increase maximum number of auto-continuations to 50 (from 30)

0 comments on commit c16980c

Please sign in to comment.