Give proposed routines a status filter and pin down the dedup rule (#46)
Look at internal/store/proposed_routines.go: status flips in place with an `AND status = 'proposed'` guard, not append-only like facts/voids_id — a proposal is a question with one answer, same shape as tools.status. The UNIQUE(action, object) key is what stops a dismissed routine coming back. New tests cover re-propose-after-dismiss and listing by status. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CGeSZxh1DCtRxmFVSYVGvJ
This commit is contained in:
@@ -8,6 +8,14 @@ import (
|
||||
"time"
|
||||
)
|
||||
|
||||
// The three states a proposal can be in. A proposal starts 'proposed' and
|
||||
// moves once, either way, and never moves again.
|
||||
const (
|
||||
RoutineProposed = "proposed"
|
||||
RoutineAccepted = "accepted"
|
||||
RoutineDismissed = "dismissed"
|
||||
)
|
||||
|
||||
// ProposedRoutine — a detected pattern the system wants to turn into a
|
||||
// recurring reminder. Status 'proposed' means awaiting human confirmation;
|
||||
// 'accepted' means the human confirmed and a reminder was created (reminder_id
|
||||
@@ -30,6 +38,15 @@ var (
|
||||
// CreateProposedRoutine inserts a new proposed routine. Returns
|
||||
// ErrProposedRoutineExists if one already exists for this action+object (any
|
||||
// status) — the pattern detector should only propose once per pair.
|
||||
//
|
||||
// action+object is the "same routine" key. It is UNIQUE in the table, so a
|
||||
// routine the human already dismissed can never come back: the detector will
|
||||
// keep finding the pattern, and every re-propose is refused here. Maven is not
|
||||
// a nag.
|
||||
//
|
||||
// TODO(vikunja#46): the detector currently only writes here from the voice
|
||||
// path. Once digestion runs the detector on its own tick, that tick should
|
||||
// call this too, so a pattern gets noticed even with nobody at the mic.
|
||||
func (s *Store) CreateProposedRoutine(ctx context.Context, action, object string, intervalDays float64, ts time.Time) (int64, error) {
|
||||
res, err := s.db.ExecContext(ctx,
|
||||
`INSERT INTO proposed_routines (action, object, interval_days, status, created_ts)
|
||||
@@ -70,14 +87,29 @@ func (s *Store) LookupProposedRoutine(ctx context.Context, action, object string
|
||||
return &r, nil
|
||||
}
|
||||
|
||||
// ListProposedRoutines returns all proposed routines with status='proposed',
|
||||
// newest first.
|
||||
// ListProposedRoutines returns the routines still waiting for an answer,
|
||||
// newest first. This is what the /routines page shows.
|
||||
func (s *Store) ListProposedRoutines(ctx context.Context) ([]ProposedRoutine, error) {
|
||||
rows, err := s.db.QueryContext(ctx, `
|
||||
SELECT id, action, object, interval_days, status, created_ts, reminder_id
|
||||
FROM proposed_routines
|
||||
WHERE status = 'proposed'
|
||||
ORDER BY created_ts DESC, id DESC`)
|
||||
return s.ListProposedRoutinesByStatus(ctx, RoutineProposed)
|
||||
}
|
||||
|
||||
// ListProposedRoutinesByStatus returns routines in one status, newest first.
|
||||
// An empty status returns every row.
|
||||
//
|
||||
// TODO(vikunja#46): the tick loop should read the accepted ones from here so a
|
||||
// routine the human said yes to has a home the loop can see, instead of only
|
||||
// the reminder row that accepting happened to create.
|
||||
func (s *Store) ListProposedRoutinesByStatus(ctx context.Context, status string) ([]ProposedRoutine, error) {
|
||||
q := `SELECT id, action, object, interval_days, status, created_ts, reminder_id
|
||||
FROM proposed_routines`
|
||||
var args []any
|
||||
if status != "" {
|
||||
q += ` WHERE status = ?`
|
||||
args = append(args, status)
|
||||
}
|
||||
q += ` ORDER BY created_ts DESC, id DESC`
|
||||
|
||||
rows, err := s.db.QueryContext(ctx, q, args...)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("list proposed routines: %w", err)
|
||||
}
|
||||
@@ -93,8 +125,19 @@ func (s *Store) ListProposedRoutines(ctx context.Context) ([]ProposedRoutine, er
|
||||
return out, rows.Err()
|
||||
}
|
||||
|
||||
// Status changes below are an in-place UPDATE, on purpose. Facts are
|
||||
// append-only (a correction writes a new row and sets voids_id) because a fact
|
||||
// is a claim about the world and the old claim is still history worth keeping.
|
||||
// A proposal is not a claim, it is a question with one answer, and the same
|
||||
// shape already exists for tools (tools.status flips in place). The guard
|
||||
// `AND status = 'proposed'` makes the move one-way: an answered proposal can
|
||||
// never be answered again.
|
||||
//
|
||||
// AcceptProposedRoutine flips status to 'accepted', links a reminder_id.
|
||||
// Returns error if not in 'proposed' status.
|
||||
//
|
||||
// TODO(vikunja#46): the /routines page calls this through ipc to flip status
|
||||
// from the authed surface.
|
||||
func (s *Store) AcceptProposedRoutine(ctx context.Context, id, reminderID int64) error {
|
||||
res, err := s.db.ExecContext(ctx,
|
||||
`UPDATE proposed_routines SET status = 'accepted', reminder_id = ? WHERE id = ? AND status = 'proposed'`,
|
||||
|
||||
@@ -131,6 +131,96 @@ func TestListProposedRoutines(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// A dismissed routine must never be proposed again. The detector will keep
|
||||
// finding the same pattern; the store is what stops maven nagging about it.
|
||||
func TestDismissedProposedRoutineStaysDismissed(t *testing.T) {
|
||||
s := newTestStore(t)
|
||||
ctx := context.Background()
|
||||
now := time.Now().UTC()
|
||||
|
||||
id, err := s.CreateProposedRoutine(ctx, "clean", "litter_box", 3.0, now)
|
||||
if err != nil {
|
||||
t.Fatalf("CreateProposedRoutine: %v", err)
|
||||
}
|
||||
if err := s.DismissProposedRoutine(ctx, id); err != nil {
|
||||
t.Fatalf("DismissProposedRoutine: %v", err)
|
||||
}
|
||||
|
||||
// The detector re-proposes the same pattern.
|
||||
_, err = s.CreateProposedRoutine(ctx, "clean", "litter_box", 3.0, now.Add(24*time.Hour))
|
||||
if !errors.Is(err, ErrProposedRoutineExists) {
|
||||
t.Fatalf("want ErrProposedRoutineExists on re-propose, got %v", err)
|
||||
}
|
||||
|
||||
// And it must not reappear on the review page.
|
||||
list, err := s.ListProposedRoutines(ctx)
|
||||
if err != nil {
|
||||
t.Fatalf("ListProposedRoutines: %v", err)
|
||||
}
|
||||
if len(list) != 0 {
|
||||
t.Fatalf("want 0 proposed, got %d", len(list))
|
||||
}
|
||||
|
||||
// Dismissing again is a no-op, and accepting is refused.
|
||||
if err := s.DismissProposedRoutine(ctx, id); err != nil {
|
||||
t.Fatalf("second DismissProposedRoutine: %v", err)
|
||||
}
|
||||
if err := s.AcceptProposedRoutine(ctx, id, 1); !errors.Is(err, ErrProposedRoutineNotFound) {
|
||||
t.Fatalf("want ErrProposedRoutineNotFound accepting a dismissed routine, got %v", err)
|
||||
}
|
||||
r, err := s.LookupProposedRoutine(ctx, "clean", "litter_box")
|
||||
if err != nil {
|
||||
t.Fatalf("LookupProposedRoutine: %v", err)
|
||||
}
|
||||
if r.Status != RoutineDismissed {
|
||||
t.Fatalf("want status=dismissed, got %s", r.Status)
|
||||
}
|
||||
}
|
||||
|
||||
func TestListProposedRoutinesByStatus(t *testing.T) {
|
||||
s := newTestStore(t)
|
||||
ctx := context.Background()
|
||||
now := time.Now().UTC()
|
||||
|
||||
keep, err := s.CreateProposedRoutine(ctx, "water", "plants", 4.0, now)
|
||||
if err != nil {
|
||||
t.Fatalf("CreateProposedRoutine: %v", err)
|
||||
}
|
||||
drop, err := s.CreateProposedRoutine(ctx, "walk", "dog", 1.0, now.Add(time.Hour))
|
||||
if err != nil {
|
||||
t.Fatalf("CreateProposedRoutine: %v", err)
|
||||
}
|
||||
remID, err := s.CreateReminder(ctx, now.Add(4*24*time.Hour), `{"text":"water plants"}`, "")
|
||||
if err != nil {
|
||||
t.Fatalf("CreateReminder: %v", err)
|
||||
}
|
||||
if err := s.AcceptProposedRoutine(ctx, keep, remID); err != nil {
|
||||
t.Fatalf("AcceptProposedRoutine: %v", err)
|
||||
}
|
||||
if err := s.DismissProposedRoutine(ctx, drop); err != nil {
|
||||
t.Fatalf("DismissProposedRoutine: %v", err)
|
||||
}
|
||||
|
||||
cases := []struct {
|
||||
status string
|
||||
want int
|
||||
}{
|
||||
{RoutineProposed, 0},
|
||||
{RoutineAccepted, 1},
|
||||
{RoutineDismissed, 1},
|
||||
{"", 2}, // empty status ⇒ every row
|
||||
}
|
||||
for _, c := range cases {
|
||||
list, err := s.ListProposedRoutinesByStatus(ctx, c.status)
|
||||
if err != nil {
|
||||
t.Fatalf("ListProposedRoutinesByStatus(%q): %v", c.status, err)
|
||||
}
|
||||
if len(list) != c.want {
|
||||
t.Fatalf("status %q: want %d, got %d", c.status, c.want, len(list))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestLookupMissingProposedRoutine(t *testing.T) {
|
||||
s := newTestStore(t)
|
||||
ctx := context.Background()
|
||||
|
||||
Reference in New Issue
Block a user