Drop workflow kinds, deliver reviews when done, rename/delete workflows
This commit is contained in:
+56
-41
@@ -196,8 +196,8 @@ func (m *Manager) startReviews(ctx context.Context, task api.Task, snap api.Snap
|
||||
}
|
||||
|
||||
def, err := m.store.GetWorkflow(ctx, project.ReviewWorkflow)
|
||||
if err == nil && def.Kind != api.WorkflowKindReview {
|
||||
err = fmt.Errorf("it is a task workflow, not a review workflow")
|
||||
if err == nil {
|
||||
err = reviewProblem(def)
|
||||
}
|
||||
if err != nil {
|
||||
return m.store.CreateReview(ctx, api.Review{
|
||||
@@ -310,7 +310,7 @@ func (m *Manager) startReviewer(ctx context.Context, task api.Task, project api.
|
||||
slog.Error("record review that didn't start", "task", task.Id, "stage", stage, "err", rerr)
|
||||
return
|
||||
}
|
||||
go m.reviewFinished(reviewID)
|
||||
go m.reviewFinished(reviewID, true)
|
||||
}()
|
||||
|
||||
prompt, err := m.reviewPrompt(ctx, task, project, snap, def, stage, inputs)
|
||||
@@ -472,66 +472,81 @@ func (m *Manager) submitReview(ctx context.Context, l *live, raw json.RawMessage
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
if len(findings) > 0 {
|
||||
go m.forwardFindings(l.taskID, l.reviewID)
|
||||
}
|
||||
go m.reviewFinished(l.reviewID)
|
||||
go m.reviewFinished(l.reviewID, true)
|
||||
return "Review recorded. End your turn now with one line.", nil
|
||||
}
|
||||
|
||||
func (m *Manager) forwardFindings(taskID, reviewID string) {
|
||||
ctx := context.Background()
|
||||
r, err := m.store.GetReview(ctx, reviewID)
|
||||
if err != nil {
|
||||
slog.Error("load review to forward", "review", reviewID, "err", err)
|
||||
func (m *Manager) deliverFindings(ctx context.Context, r api.Review, reviews []api.Review) {
|
||||
last := r
|
||||
for _, sr := range reviews {
|
||||
if sr.FinishedAt != nil && last.FinishedAt != nil && (sr.FinishedAt.After(*last.FinishedAt) || sr.FinishedAt.Equal(*last.FinishedAt) && sr.Id > last.Id) {
|
||||
last = sr
|
||||
}
|
||||
}
|
||||
if last.Id != r.Id {
|
||||
return
|
||||
}
|
||||
s, found, err := m.liveSession(ctx, taskID)
|
||||
latest, err := m.store.LastReviewable(ctx, r.TaskId)
|
||||
if err != nil || latest.N != r.SnapshotN {
|
||||
return
|
||||
}
|
||||
text := findingsMessage(r.SnapshotN, reviews)
|
||||
if text == "" {
|
||||
return
|
||||
}
|
||||
s, found, err := m.liveSession(ctx, r.TaskId)
|
||||
if err != nil {
|
||||
slog.Error("list sessions to forward review", "review", reviewID, "err", err)
|
||||
slog.Error("list sessions to deliver findings", "task", r.TaskId, "err", err)
|
||||
return
|
||||
}
|
||||
if !found {
|
||||
slog.Info("review findings not forwarded, task has no live agent", "review", reviewID, "task", taskID)
|
||||
slog.Info("review findings not delivered, task has no live agent", "task", r.TaskId, "snapshot", r.SnapshotN)
|
||||
return
|
||||
}
|
||||
_, _, err = m.Message(ctx, s.Id, "byre", findingsMessage(r), "", "")
|
||||
_, _, err = m.Message(ctx, s.Id, "byre", text, "", "")
|
||||
if err != nil {
|
||||
slog.Error("forward review findings", "review", reviewID, "session", s.Id, "err", err)
|
||||
slog.Error("deliver review findings", "task", r.TaskId, "snapshot", r.SnapshotN, "session", s.Id, "err", err)
|
||||
return
|
||||
}
|
||||
slog.Info("review findings forwarded", "review", reviewID, "session", s.Id)
|
||||
slog.Info("review findings delivered", "task", r.TaskId, "snapshot", r.SnapshotN, "session", s.Id)
|
||||
}
|
||||
|
||||
const findingsPreamble = "A reviewer looked at snapshot #"
|
||||
const findingsPreamble = "The review of snapshot #"
|
||||
|
||||
func isFindingsMessage(actor, text string) bool {
|
||||
return actor == "byre" && strings.HasPrefix(text, findingsPreamble)
|
||||
}
|
||||
|
||||
func findingsMessage(r api.Review) string {
|
||||
res := map[string]any{}
|
||||
if r.Result != nil {
|
||||
res = *r.Result
|
||||
}
|
||||
func findingsMessage(n int, reviews []api.Review) string {
|
||||
b := &strings.Builder{}
|
||||
fmt.Fprintf(b, findingsPreamble+"%d of your work as reviewer %q and raised the findings below. Reviewers can be wrong or miss context: check each one against the code and your brief. "+
|
||||
"Before you change anything, answer each finding with respond_to_finding (snapshot %d, reviewer %q and the finding's number): fixing if it holds up and you'll act on it, ignoring with your reason if you're leaving it. "+
|
||||
"The human sees your answers on the findings and can overrule an ignore.", r.SnapshotN, r.Stage, r.SnapshotN, r.Stage)
|
||||
findings, _ := res["findings"].([]any)
|
||||
for i, f := range findings {
|
||||
fm, _ := f.(map[string]any)
|
||||
where := ""
|
||||
if p, ok := fm["path"].(string); ok && p != "" {
|
||||
where = p
|
||||
if line, ok := fm["line"].(float64); ok && line > 0 {
|
||||
where += fmt.Sprintf(":%d", int(line))
|
||||
}
|
||||
where = " (`" + where + "`)"
|
||||
for _, r := range reviews {
|
||||
if r.State != api.ReviewStateDone || r.Result == nil {
|
||||
continue
|
||||
}
|
||||
findings, _ := (*r.Result)["findings"].([]any)
|
||||
if len(findings) == 0 {
|
||||
continue
|
||||
}
|
||||
fmt.Fprintf(b, "\n\n## Reviewer %q", r.Stage)
|
||||
for i, f := range findings {
|
||||
fm, _ := f.(map[string]any)
|
||||
where := ""
|
||||
if p, ok := fm["path"].(string); ok && p != "" {
|
||||
where = p
|
||||
if line, ok := fm["line"].(float64); ok && line > 0 {
|
||||
where += fmt.Sprintf(":%d", int(line))
|
||||
}
|
||||
where = " (`" + where + "`)"
|
||||
}
|
||||
fmt.Fprintf(b, "\n\n**Finding %d: [%v] %v**%s\n\n%s", i+1, fm["severity"], fm["title"], where, strings.TrimSpace(fmt.Sprint(fm["detail"])))
|
||||
}
|
||||
fmt.Fprintf(b, "\n\n**Finding %d: [%v] %v**%s\n\n%s", i+1, fm["severity"], fm["title"], where, strings.TrimSpace(fmt.Sprint(fm["detail"])))
|
||||
}
|
||||
return b.String()
|
||||
if b.Len() == 0 {
|
||||
return ""
|
||||
}
|
||||
return fmt.Sprintf(findingsPreamble+"%d of your work has finished, and its reviewers raised the findings below. Reviewers can be wrong or miss context: check each one against the code and your brief. "+
|
||||
"Before you change anything, answer each finding with respond_to_finding (snapshot %d, the reviewer's name and the finding's number): fixing if it holds up and you'll act on it, ignoring with your reason if you're leaving it. "+
|
||||
"The human sees your answers on the findings and can overrule an ignore.", n, n) + b.String()
|
||||
}
|
||||
|
||||
type findingResponseArgs struct {
|
||||
@@ -722,7 +737,7 @@ func (m *Manager) failReview(ctx context.Context, reviewID, reason string) {
|
||||
return
|
||||
}
|
||||
slog.Info("review failed", "review", reviewID, "reason", reason)
|
||||
go m.reviewFinished(reviewID)
|
||||
go m.reviewFinished(reviewID, true)
|
||||
}
|
||||
|
||||
func (m *Manager) cancelReviews(ctx context.Context, task api.Task, project api.Project) {
|
||||
@@ -824,7 +839,7 @@ func (m *Manager) recoverReviews(ctx context.Context) error {
|
||||
return err
|
||||
}
|
||||
for _, id := range done {
|
||||
go m.reviewFinished(id)
|
||||
go m.reviewFinished(id, false)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user