Add configurable sort policy for GetReadyWork (bd-147)
- Add SortPolicy type with hybrid, priority, oldest constants - Add SortPolicy field to WorkFilter - Implement buildOrderByClause() for SQL generation - Add --sort flag to bd ready command - Add comprehensive tests for all 3 sort policies - Update RPC protocol to support sort policy - Update documentation with sort policy examples Enables autonomous systems like VC to use strict priority ordering while preserving hybrid behavior for interactive use. Amp-Thread-ID: https://ampcode.com/threads/T-9d7ea9db-8d6d-4498-9daa-48a7e104ce1f Co-authored-by: Amp <amp@ampcode.com>
This commit is contained in:
@@ -44,6 +44,13 @@ func (s *SQLiteStorage) GetReadyWork(ctx context.Context, filter types.WorkFilte
|
||||
args = append(args, filter.Limit)
|
||||
}
|
||||
|
||||
// Default to hybrid sort for backwards compatibility
|
||||
sortPolicy := filter.SortPolicy
|
||||
if sortPolicy == "" {
|
||||
sortPolicy = types.SortPolicyHybrid
|
||||
}
|
||||
orderBySQL := buildOrderByClause(sortPolicy)
|
||||
|
||||
// Query with recursive CTE to propagate blocking through parent-child hierarchy
|
||||
// Algorithm:
|
||||
// 1. Find issues directly blocked by 'blocks' dependencies
|
||||
@@ -85,23 +92,9 @@ func (s *SQLiteStorage) GetReadyWork(ctx context.Context, filter types.WorkFilte
|
||||
AND NOT EXISTS (
|
||||
SELECT 1 FROM blocked_transitively WHERE issue_id = i.id
|
||||
)
|
||||
ORDER BY
|
||||
-- Hybrid sort: recent issues (48 hours) by priority, then oldest-first
|
||||
CASE
|
||||
WHEN datetime(i.created_at) >= datetime('now', '-48 hours') THEN 0
|
||||
ELSE 1
|
||||
END ASC,
|
||||
CASE
|
||||
WHEN datetime(i.created_at) >= datetime('now', '-48 hours') THEN i.priority
|
||||
ELSE NULL
|
||||
END ASC,
|
||||
CASE
|
||||
WHEN datetime(i.created_at) < datetime('now', '-48 hours') THEN i.created_at
|
||||
ELSE NULL
|
||||
END ASC,
|
||||
i.created_at ASC
|
||||
%s
|
||||
`, whereSQL, limitSQL)
|
||||
%s
|
||||
%s
|
||||
`, whereSQL, orderBySQL, limitSQL)
|
||||
|
||||
rows, err := s.db.QueryContext(ctx, query, args...)
|
||||
if err != nil {
|
||||
@@ -180,3 +173,32 @@ func (s *SQLiteStorage) GetBlockedIssues(ctx context.Context) ([]*types.BlockedI
|
||||
|
||||
return blocked, nil
|
||||
}
|
||||
|
||||
// buildOrderByClause generates the ORDER BY clause based on sort policy
|
||||
func buildOrderByClause(policy types.SortPolicy) string {
|
||||
switch policy {
|
||||
case types.SortPolicyPriority:
|
||||
return `ORDER BY i.priority ASC, i.created_at ASC`
|
||||
|
||||
case types.SortPolicyOldest:
|
||||
return `ORDER BY i.created_at ASC`
|
||||
|
||||
case types.SortPolicyHybrid:
|
||||
fallthrough
|
||||
default:
|
||||
return `ORDER BY
|
||||
CASE
|
||||
WHEN datetime(i.created_at) >= datetime('now', '-48 hours') THEN 0
|
||||
ELSE 1
|
||||
END ASC,
|
||||
CASE
|
||||
WHEN datetime(i.created_at) >= datetime('now', '-48 hours') THEN i.priority
|
||||
ELSE NULL
|
||||
END ASC,
|
||||
CASE
|
||||
WHEN datetime(i.created_at) < datetime('now', '-48 hours') THEN i.created_at
|
||||
ELSE NULL
|
||||
END ASC,
|
||||
i.created_at ASC`
|
||||
}
|
||||
}
|
||||
|
||||
@@ -916,3 +916,181 @@ func TestExplainQueryPlanReadyWork(t *testing.T) {
|
||||
t.Error("Query plan contains table scans - indexes may not be used efficiently")
|
||||
}
|
||||
}
|
||||
|
||||
// TestSortPolicyPriority tests strict priority-first sorting
|
||||
func TestSortPolicyPriority(t *testing.T) {
|
||||
store, cleanup := setupTestDB(t)
|
||||
defer cleanup()
|
||||
|
||||
ctx := context.Background()
|
||||
|
||||
// Create issues with mixed ages and priorities
|
||||
// Old issues (72 hours ago)
|
||||
issueP0Old := &types.Issue{Title: "old-P0", Status: types.StatusOpen, Priority: 0, IssueType: types.TypeTask}
|
||||
issueP2Old := &types.Issue{Title: "old-P2", Status: types.StatusOpen, Priority: 2, IssueType: types.TypeTask}
|
||||
issueP1Old := &types.Issue{Title: "old-P1", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask}
|
||||
|
||||
// Recent issues (12 hours ago)
|
||||
issueP3New := &types.Issue{Title: "new-P3", Status: types.StatusOpen, Priority: 3, IssueType: types.TypeTask}
|
||||
issueP1New := &types.Issue{Title: "new-P1", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask}
|
||||
|
||||
// Create old issues first (to have older created_at)
|
||||
store.CreateIssue(ctx, issueP0Old, "test-user")
|
||||
store.CreateIssue(ctx, issueP2Old, "test-user")
|
||||
store.CreateIssue(ctx, issueP1Old, "test-user")
|
||||
|
||||
// Create new issues
|
||||
store.CreateIssue(ctx, issueP3New, "test-user")
|
||||
store.CreateIssue(ctx, issueP1New, "test-user")
|
||||
|
||||
// Use priority sort policy
|
||||
ready, err := store.GetReadyWork(ctx, types.WorkFilter{
|
||||
Status: types.StatusOpen,
|
||||
SortPolicy: types.SortPolicyPriority,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("GetReadyWork failed: %v", err)
|
||||
}
|
||||
|
||||
if len(ready) != 5 {
|
||||
t.Fatalf("Expected 5 ready issues, got %d", len(ready))
|
||||
}
|
||||
|
||||
// Verify strict priority ordering: P0, P1, P1, P2, P3
|
||||
// Within same priority, older created_at comes first
|
||||
expectedOrder := []struct {
|
||||
title string
|
||||
priority int
|
||||
}{
|
||||
{"old-P0", 0},
|
||||
{"old-P1", 1},
|
||||
{"new-P1", 1},
|
||||
{"old-P2", 2},
|
||||
{"new-P3", 3},
|
||||
}
|
||||
|
||||
for i, expected := range expectedOrder {
|
||||
if ready[i].Title != expected.title {
|
||||
t.Errorf("Position %d: expected %s, got %s", i, expected.title, ready[i].Title)
|
||||
}
|
||||
if ready[i].Priority != expected.priority {
|
||||
t.Errorf("Position %d: expected P%d, got P%d", i, expected.priority, ready[i].Priority)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestSortPolicyOldest tests oldest-first sorting (ignoring priority)
|
||||
func TestSortPolicyOldest(t *testing.T) {
|
||||
store, cleanup := setupTestDB(t)
|
||||
defer cleanup()
|
||||
|
||||
ctx := context.Background()
|
||||
|
||||
// Create issues in order: P2, P0, P1 (mixed priority, chronological creation)
|
||||
issueP2 := &types.Issue{Title: "first-P2", Status: types.StatusOpen, Priority: 2, IssueType: types.TypeTask}
|
||||
issueP0 := &types.Issue{Title: "second-P0", Status: types.StatusOpen, Priority: 0, IssueType: types.TypeTask}
|
||||
issueP1 := &types.Issue{Title: "third-P1", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask}
|
||||
|
||||
store.CreateIssue(ctx, issueP2, "test-user")
|
||||
store.CreateIssue(ctx, issueP0, "test-user")
|
||||
store.CreateIssue(ctx, issueP1, "test-user")
|
||||
|
||||
// Use oldest sort policy
|
||||
ready, err := store.GetReadyWork(ctx, types.WorkFilter{
|
||||
Status: types.StatusOpen,
|
||||
SortPolicy: types.SortPolicyOldest,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("GetReadyWork failed: %v", err)
|
||||
}
|
||||
|
||||
if len(ready) != 3 {
|
||||
t.Fatalf("Expected 3 ready issues, got %d", len(ready))
|
||||
}
|
||||
|
||||
// Should be sorted by creation time only (oldest first)
|
||||
expectedTitles := []string{"first-P2", "second-P0", "third-P1"}
|
||||
for i, expected := range expectedTitles {
|
||||
if ready[i].Title != expected {
|
||||
t.Errorf("Position %d: expected %s, got %s", i, expected, ready[i].Title)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestSortPolicyHybrid tests hybrid sort (default behavior)
|
||||
func TestSortPolicyHybrid(t *testing.T) {
|
||||
store, cleanup := setupTestDB(t)
|
||||
defer cleanup()
|
||||
|
||||
ctx := context.Background()
|
||||
|
||||
// Create issues with different priorities
|
||||
// All created recently (within 48 hours in test), so should sort by priority
|
||||
issueP0 := &types.Issue{Title: "issue-P0", Status: types.StatusOpen, Priority: 0, IssueType: types.TypeTask}
|
||||
issueP2 := &types.Issue{Title: "issue-P2", Status: types.StatusOpen, Priority: 2, IssueType: types.TypeTask}
|
||||
issueP1 := &types.Issue{Title: "issue-P1", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask}
|
||||
issueP3 := &types.Issue{Title: "issue-P3", Status: types.StatusOpen, Priority: 3, IssueType: types.TypeTask}
|
||||
|
||||
store.CreateIssue(ctx, issueP2, "test-user")
|
||||
store.CreateIssue(ctx, issueP0, "test-user")
|
||||
store.CreateIssue(ctx, issueP3, "test-user")
|
||||
store.CreateIssue(ctx, issueP1, "test-user")
|
||||
|
||||
// Use hybrid sort policy (explicit)
|
||||
ready, err := store.GetReadyWork(ctx, types.WorkFilter{
|
||||
Status: types.StatusOpen,
|
||||
SortPolicy: types.SortPolicyHybrid,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("GetReadyWork failed: %v", err)
|
||||
}
|
||||
|
||||
if len(ready) != 4 {
|
||||
t.Fatalf("Expected 4 ready issues, got %d", len(ready))
|
||||
}
|
||||
|
||||
// Since all issues are created recently (< 48 hours in test context),
|
||||
// hybrid sort should order by priority: P0, P1, P2, P3
|
||||
expectedPriorities := []int{0, 1, 2, 3}
|
||||
for i, expected := range expectedPriorities {
|
||||
if ready[i].Priority != expected {
|
||||
t.Errorf("Position %d: expected P%d, got P%d", i, expected, ready[i].Priority)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestSortPolicyDefault tests that empty sort policy defaults to hybrid
|
||||
func TestSortPolicyDefault(t *testing.T) {
|
||||
store, cleanup := setupTestDB(t)
|
||||
defer cleanup()
|
||||
|
||||
ctx := context.Background()
|
||||
|
||||
// Create test issues with different priorities
|
||||
issueP1 := &types.Issue{Title: "issue-P1", Status: types.StatusOpen, Priority: 1, IssueType: types.TypeTask}
|
||||
issueP2 := &types.Issue{Title: "issue-P2", Status: types.StatusOpen, Priority: 2, IssueType: types.TypeTask}
|
||||
|
||||
store.CreateIssue(ctx, issueP2, "test-user")
|
||||
store.CreateIssue(ctx, issueP1, "test-user")
|
||||
|
||||
// Use default (empty) sort policy
|
||||
ready, err := store.GetReadyWork(ctx, types.WorkFilter{
|
||||
Status: types.StatusOpen,
|
||||
// SortPolicy not specified - should default to hybrid
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("GetReadyWork failed: %v", err)
|
||||
}
|
||||
|
||||
if len(ready) != 2 {
|
||||
t.Fatalf("Expected 2 ready issues, got %d", len(ready))
|
||||
}
|
||||
|
||||
// Should behave like hybrid: since both are recent, sort by priority (P1 first)
|
||||
if ready[0].Priority != 1 {
|
||||
t.Errorf("Expected P1 first (hybrid default, recent by priority), got P%d", ready[0].Priority)
|
||||
}
|
||||
if ready[1].Priority != 2 {
|
||||
t.Errorf("Expected P2 second, got P%d", ready[1].Priority)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user