From 8ac9ae154c74fb249e329b50eb89dca9d08736f8 Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Mon, 9 Oct 2023 10:38:04 +0200
Subject: [PATCH 01/41] feat: add notifiable observable

Co-authored-by: red-0ne <r3d0ne@gmail.com>
---
 pkg/observable/interface.go                  |  16 ++
 pkg/observable/notifiable/observable.go      |  93 +++++++++++
 pkg/observable/notifiable/observable_test.go | 162 +++++++++++++++++++
 pkg/observable/notifiable/subscription.go    |  29 ++++
 4 files changed, 300 insertions(+)
 create mode 100644 pkg/observable/interface.go
 create mode 100644 pkg/observable/notifiable/observable.go
 create mode 100644 pkg/observable/notifiable/observable_test.go
 create mode 100644 pkg/observable/notifiable/subscription.go

diff --git a/pkg/observable/interface.go b/pkg/observable/interface.go
new file mode 100644
index 000000000..0b7fbe337
--- /dev/null
+++ b/pkg/observable/interface.go
@@ -0,0 +1,16 @@
+package observable
+
+import "context"
+
+// Observable is a generic interface that allows multiple subscribers to be
+// notified of new values asynchronously.
+type Observable[V any] interface {
+	Subscribe(context.Context) Subscription[V]
+}
+
+// Subscription is a generic interface that provides access to the notified
+// channel and allows unsubscribing from an observable.
+type Subscription[V any] interface {
+	Unsubscribe()
+	Ch() <-chan V
+}
diff --git a/pkg/observable/notifiable/observable.go b/pkg/observable/notifiable/observable.go
new file mode 100644
index 000000000..da9433805
--- /dev/null
+++ b/pkg/observable/notifiable/observable.go
@@ -0,0 +1,93 @@
+package notifiable
+
+import (
+	"context"
+	"sync"
+
+	"pocket/pkg/observable"
+)
+
+// Observable implements the observable.Observable interface and can be notified
+// via its corresponding notifier channel.
+type Observable[V any] struct {
+	mu          sync.RWMutex
+	ch          <-chan V // private channel that is used to emit values to subscribers
+	subscribers []chan V // subscribers is a list of channels that are subscribed to the observable
+	closed      bool
+}
+
+// NewObservable creates a new observable is notified when the notifier channel
+// receives a value.
+func NewObservable[V any](notifier chan V) (observable.Observable[V], chan V) {
+	// If the caller does not provide a notifier, create a new one and return it
+	if notifier == nil {
+		notifier = make(chan V)
+	}
+	notifee := &Observable[V]{sync.RWMutex{}, notifier, []chan V{}, false}
+
+	// Start listening to the notifier and emit values to subscribers
+	go notifee.listen(notifier)
+
+	return notifee, notifier
+}
+
+// Subscribe gets a subscription to the observable.
+func (obs *Observable[V]) Subscribe(ctx context.Context) observable.Subscription[V] {
+	obs.mu.Lock()
+	defer obs.mu.Unlock()
+
+	// Create a channel for the subscriber and append it to the subscribers list
+	ch := make(chan V, 1)
+	obs.subscribers = append(obs.subscribers, ch)
+
+	// Removal function used when unsubscribing from the observable
+	removeFromObservable := func() {
+		obs.mu.Lock()
+		defer obs.mu.Unlock()
+
+		for i, s := range obs.subscribers {
+			if ch == s {
+				obs.subscribers = append(obs.subscribers[:i], obs.subscribers[i+1:]...)
+				break
+			}
+		}
+	}
+
+	// Subscription gets its closed state from the observable
+	subscription := &Subscription[V]{ch, obs.closed, removeFromObservable}
+
+	go unsubscribeOnDone[V](ctx, subscription)
+
+	return subscription
+}
+
+// listen to the notifier and notify subscribers when values are received. This
+// function is blocking and should be run in a goroutine.
+func (obs *Observable[V]) listen(notifier <-chan V) {
+	for v := range notifier {
+		// Lock for obs.subscribers slice as it can be modified by subscribers
+		obs.mu.RLock()
+		for _, ch := range obs.subscribers {
+			ch <- v
+		}
+		obs.mu.RUnlock()
+	}
+
+	// Here we know that the notifier has been closed, all subscribers should be closed as well
+	obs.mu.Lock()
+	obs.closed = true
+	for _, ch := range obs.subscribers {
+		close(ch)
+		obs.subscribers = []chan V{}
+	}
+	obs.mu.Lock()
+}
+
+// unsubscribeOnDone unsubscribes from the subscription when the context is.
+// It is blocking and intended to be called in a goroutine.
+func unsubscribeOnDone[V any](ctx context.Context, subscription observable.Subscription[V]) {
+	if ctx != nil {
+		<-ctx.Done()
+		subscription.Unsubscribe()
+	}
+}
diff --git a/pkg/observable/notifiable/observable_test.go b/pkg/observable/notifiable/observable_test.go
new file mode 100644
index 000000000..f3aa0bc23
--- /dev/null
+++ b/pkg/observable/notifiable/observable_test.go
@@ -0,0 +1,162 @@
+package notifiable_test
+
+import (
+	"context"
+	"fmt"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+	"golang.org/x/sync/errgroup"
+
+	"pocket/pkg/observable"
+	"pocket/pkg/observable/notifiable"
+)
+
+const (
+	notifyTimeout            = 100 * time.Millisecond
+	unsubscribeSleepDuration = notifyTimeout * 2
+)
+
+func TestNewNotifiableObservable(t *testing.T) {
+	type test struct {
+		name     string
+		notifier chan int
+	}
+
+	input := 123
+	nonEmptyBufferedNotifier := make(chan int, 1)
+	nonEmptyBufferedNotifier <- input
+
+	tests := []test{
+		{name: "nil notifier", notifier: nil},
+		{name: "empty non-buffered notifier", notifier: make(chan int)},
+		{name: "empty buffered len 1 notifier", notifier: make(chan int, 1)},
+		{name: "non-empty buffered notifier", notifier: nonEmptyBufferedNotifier},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ctx := context.Background()
+			notifee, notifier := notifiable.NewObservable[int](tt.notifier)
+			require.NotNil(t, notifee)
+			require.NotNil(t, notifier)
+
+			// construct 3 distinct subscriptions, each with its own channel
+			subscriptions := make([]observable.Subscription[int], 3)
+			for i := range subscriptions {
+				subscriptions[i] = notifee.Subscribe(ctx)
+			}
+
+			group := errgroup.Group{}
+			notifiedOrTimedOut := func(subscriptionCh <-chan int) func() error {
+				return func() error {
+					// subscriptionCh should receive notified input
+					select {
+					case output := <-subscriptionCh:
+						require.Equal(t, input, output)
+					case <-time.After(notifyTimeout):
+						return fmt.Errorf("timed out waiting for subscription to be notified")
+					}
+					return nil
+				}
+			}
+
+			// ensure all subscription channels are notified
+			for _, subscription := range subscriptions {
+				// concurrently await notification or timeout to avoid blocking on
+				// empty and/or non-buffered notifiers.
+				group.Go(notifiedOrTimedOut(subscription.Ch()))
+			}
+
+			// notify with test input
+			notifier <- input
+
+			// wait for notifee to be notified or timeout
+			err := group.Wait()
+			require.NoError(t, err)
+
+			// unsubscribing should close subscription channel(s)
+			for _, subscription := range subscriptions {
+				subscription.Unsubscribe()
+
+				select {
+				case <-subscription.Ch():
+				default:
+					t.Fatal("subscription channel left open")
+				}
+			}
+		})
+	}
+}
+
+func TestSubscription_Unsubscribe(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	notifee, notifier := notifiable.NewObservable[int](nil)
+	require.NotNil(t, notifee)
+	require.NotNil(t, notifier)
+
+	tests := []struct {
+		name        string
+		lifecycleFn func() observable.Subscription[int]
+	}{
+		{
+			name: "nil context",
+			lifecycleFn: func() observable.Subscription[int] {
+				subscription := notifee.Subscribe(nil)
+				subscription.Unsubscribe()
+				return subscription
+			},
+		},
+		{
+			name: "only unsubscribe",
+			lifecycleFn: func() observable.Subscription[int] {
+				subscription := notifee.Subscribe(ctx)
+				subscription.Unsubscribe()
+				return subscription
+			},
+		},
+		{
+			name: "only cancel",
+			lifecycleFn: func() observable.Subscription[int] {
+				subscription := notifee.Subscribe(ctx)
+				cancel()
+				return subscription
+			},
+		},
+		{
+			name: "cancel then unsubscribe",
+			lifecycleFn: func() observable.Subscription[int] {
+				subscription := notifee.Subscribe(ctx)
+				cancel()
+				time.Sleep(unsubscribeSleepDuration)
+				subscription.Unsubscribe()
+				return subscription
+			},
+		},
+		{
+			name: "unsubscribe then cancel",
+			lifecycleFn: func() observable.Subscription[int] {
+				subscription := notifee.Subscribe(ctx)
+				subscription.Unsubscribe()
+				time.Sleep(unsubscribeSleepDuration)
+				cancel()
+				return subscription
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			subscription := tt.lifecycleFn()
+
+			select {
+			case value, ok := <-subscription.Ch():
+				require.Empty(t, value)
+				require.False(t, ok)
+			case <-time.After(notifyTimeout):
+				t.Fatal("subscription channel left open")
+			}
+		})
+	}
+}
diff --git a/pkg/observable/notifiable/subscription.go b/pkg/observable/notifiable/subscription.go
new file mode 100644
index 000000000..f31f31bbe
--- /dev/null
+++ b/pkg/observable/notifiable/subscription.go
@@ -0,0 +1,29 @@
+package notifiable
+
+import "pocket/pkg/observable"
+
+var _ observable.Subscription[any] = &Subscription[any]{}
+
+// Subscription implements the observable.Subscription interface.
+type Subscription[V any] struct {
+	ch                   chan V
+	closed               bool
+	removeFromObservable func()
+}
+
+// Unsubscribe closes the subscription channel and removes the subscription from
+// the observable.
+func (sub *Subscription[V]) Unsubscribe() {
+	if sub.closed {
+		return
+	}
+
+	close(sub.ch)
+	sub.closed = true
+	sub.removeFromObservable()
+}
+
+// Ch returns the subscription channel.
+func (sub *Subscription[V]) Ch() <-chan V {
+	return sub.ch
+}

From 316db40cb02532b3f4875599746f47acc2adc271 Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Tue, 10 Oct 2023 20:29:40 +0200
Subject: [PATCH 02/41] fixup: observable

(cherry picked from commit bcf700405b5e4bd71bf9bb650c988526fa16c728)
---
 pkg/observable/notifiable/observable.go   | 10 ++++++----
 pkg/observable/notifiable/subscription.go | 10 +++++++++-
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/pkg/observable/notifiable/observable.go b/pkg/observable/notifiable/observable.go
index da9433805..51c955a69 100644
--- a/pkg/observable/notifiable/observable.go
+++ b/pkg/observable/notifiable/observable.go
@@ -42,9 +42,6 @@ func (obs *Observable[V]) Subscribe(ctx context.Context) observable.Subscription
 
 	// Removal function used when unsubscribing from the observable
 	removeFromObservable := func() {
-		obs.mu.Lock()
-		defer obs.mu.Unlock()
-
 		for i, s := range obs.subscribers {
 			if ch == s {
 				obs.subscribers = append(obs.subscribers[:i], obs.subscribers[i+1:]...)
@@ -54,7 +51,12 @@ func (obs *Observable[V]) Subscribe(ctx context.Context) observable.Subscription
 	}
 
 	// Subscription gets its closed state from the observable
-	subscription := &Subscription[V]{ch, obs.closed, removeFromObservable}
+	subscription := &Subscription[V]{
+		mu:                   &obs.mu,
+		ch:                   ch,
+		closed:               obs.closed,
+		removeFromObservable: removeFromObservable,
+	}
 
 	go unsubscribeOnDone[V](ctx, subscription)
 
diff --git a/pkg/observable/notifiable/subscription.go b/pkg/observable/notifiable/subscription.go
index f31f31bbe..32fce865c 100644
--- a/pkg/observable/notifiable/subscription.go
+++ b/pkg/observable/notifiable/subscription.go
@@ -1,11 +1,16 @@
 package notifiable
 
-import "pocket/pkg/observable"
+import (
+	"sync"
+
+	"pocket/pkg/observable"
+)
 
 var _ observable.Subscription[any] = &Subscription[any]{}
 
 // Subscription implements the observable.Subscription interface.
 type Subscription[V any] struct {
+	mu                   *sync.RWMutex
 	ch                   chan V
 	closed               bool
 	removeFromObservable func()
@@ -14,6 +19,9 @@ type Subscription[V any] struct {
 // Unsubscribe closes the subscription channel and removes the subscription from
 // the observable.
 func (sub *Subscription[V]) Unsubscribe() {
+	sub.mu.Lock()
+	defer sub.mu.Unlock()
+
 	if sub.closed {
 		return
 	}

From e50b83cccb5d3aa9bf91493ef881a364c9cf1997 Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Thu, 12 Oct 2023 15:50:31 +0200
Subject: [PATCH 03/41] refactor/fix: notifiable observable improvements

---
 go.mod                                       |   4 +-
 pkg/observable/interface.go                  |   7 +-
 pkg/observable/notifiable/observable.go      | 126 +++++-----
 pkg/observable/notifiable/observable_test.go | 229 +++++++++++++------
 pkg/observable/notifiable/observer.go        |  94 ++++++++
 pkg/observable/notifiable/observer_test.go   |  34 +++
 pkg/observable/notifiable/subscription.go    |  37 ---
 7 files changed, 373 insertions(+), 158 deletions(-)
 create mode 100644 pkg/observable/notifiable/observer.go
 create mode 100644 pkg/observable/notifiable/observer_test.go
 delete mode 100644 pkg/observable/notifiable/subscription.go

diff --git a/go.mod b/go.mod
index 5d310b203..e96203024 100644
--- a/go.mod
+++ b/go.mod
@@ -17,7 +17,7 @@ require (
 	github.com/spf13/cobra v1.7.0
 	github.com/spf13/pflag v1.0.5
 	github.com/stretchr/testify v1.8.4
-	google.golang.org/genproto v0.0.0-20230410155749-daa745c078e1
+	golang.org/x/sync v0.3.0
 	google.golang.org/grpc v1.56.1
 	gopkg.in/yaml.v2 v2.4.0
 )
@@ -250,7 +250,6 @@ require (
 	golang.org/x/mod v0.11.0 // indirect
 	golang.org/x/net v0.14.0 // indirect
 	golang.org/x/oauth2 v0.7.0 // indirect
-	golang.org/x/sync v0.3.0 // indirect
 	golang.org/x/sys v0.11.0 // indirect
 	golang.org/x/term v0.11.0 // indirect
 	golang.org/x/text v0.12.0 // indirect
@@ -259,6 +258,7 @@ require (
 	gonum.org/v1/gonum v0.11.0 // indirect
 	google.golang.org/api v0.122.0 // indirect
 	google.golang.org/appengine v1.6.7 // indirect
+	google.golang.org/genproto v0.0.0-20230410155749-daa745c078e1 // indirect
 	google.golang.org/protobuf v1.31.0 // indirect
 	gopkg.in/ini.v1 v1.67.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
diff --git a/pkg/observable/interface.go b/pkg/observable/interface.go
index 0b7fbe337..8e53e914d 100644
--- a/pkg/observable/interface.go
+++ b/pkg/observable/interface.go
@@ -5,12 +5,13 @@ import "context"
 // Observable is a generic interface that allows multiple subscribers to be
 // notified of new values asynchronously.
 type Observable[V any] interface {
-	Subscribe(context.Context) Subscription[V]
+	Subscribe(context.Context) Observer[V]
+	Close()
 }
 
-// Subscription is a generic interface that provides access to the notified
+// Observer is a generic interface that provides access to the notified
 // channel and allows unsubscribing from an observable.
-type Subscription[V any] interface {
+type Observer[V any] interface {
 	Unsubscribe()
 	Ch() <-chan V
 }
diff --git a/pkg/observable/notifiable/observable.go b/pkg/observable/notifiable/observable.go
index 51c955a69..f519f0774 100644
--- a/pkg/observable/notifiable/observable.go
+++ b/pkg/observable/notifiable/observable.go
@@ -2,18 +2,22 @@ package notifiable
 
 import (
 	"context"
+	"fmt"
 	"sync"
 
 	"pocket/pkg/observable"
 )
 
-// Observable implements the observable.Observable interface and can be notified
+var _ observable.Observable[any] = &notifiableObservable[any]{}
+
+// notifiableObservable implements the observable.Observable interface and can be notified
 // via its corresponding notifier channel.
-type Observable[V any] struct {
-	mu          sync.RWMutex
-	ch          <-chan V // private channel that is used to emit values to subscribers
-	subscribers []chan V // subscribers is a list of channels that are subscribed to the observable
-	closed      bool
+type notifiableObservable[V any] struct {
+	notifier    <-chan V // private channel that is used to emit values to observers
+	observersMu sync.RWMutex
+	// TODO_THIS_COMMIT: update comment(s)
+	// TODO_THIS_COMMIT: consider using interface type
+	observers *[]*observer[V] // observers is a list of channels that are subscribed to the observable
 }
 
 // NewObservable creates a new observable is notified when the notifier channel
@@ -23,73 +27,91 @@ func NewObservable[V any](notifier chan V) (observable.Observable[V], chan V) {
 	if notifier == nil {
 		notifier = make(chan V)
 	}
-	notifee := &Observable[V]{sync.RWMutex{}, notifier, []chan V{}, false}
+	observable := &notifiableObservable[V]{
+		notifier,
+		sync.RWMutex{},
+		new([]*observer[V]),
+	}
 
-	// Start listening to the notifier and emit values to subscribers
-	go notifee.listen(notifier)
+	// Start listening to the notifier and emit values to observers
+	go observable.listen(notifier)
 
-	return notifee, notifier
+	return observable, notifier
 }
 
-// Subscribe gets a subscription to the observable.
-func (obs *Observable[V]) Subscribe(ctx context.Context) observable.Subscription[V] {
-	obs.mu.Lock()
-	defer obs.mu.Unlock()
+// Subscribe returns an observer which is notified when notifier receives.
+func (obs *notifiableObservable[V]) Subscribe(ctx context.Context) observable.Observer[V] {
+	obs.observersMu.Lock()
+	defer func() {
+		obs.observersMu.Unlock()
+	}()
 
-	// Create a channel for the subscriber and append it to the subscribers list
-	ch := make(chan V, 1)
-	obs.subscribers = append(obs.subscribers, ch)
+	observer := NewSubscription[V](ctx, obs.onUnsubscribeFactory)
 
-	// Removal function used when unsubscribing from the observable
-	removeFromObservable := func() {
-		for i, s := range obs.subscribers {
-			if ch == s {
-				obs.subscribers = append(obs.subscribers[:i], obs.subscribers[i+1:]...)
-				break
-			}
-		}
-	}
+	go unsubscribeOnDone[V](ctx, observer)
+	return observer
+}
 
-	// Subscription gets its closed state from the observable
-	subscription := &Subscription[V]{
-		mu:                   &obs.mu,
-		ch:                   ch,
-		closed:               obs.closed,
-		removeFromObservable: removeFromObservable,
-	}
+func (obs *notifiableObservable[V]) Close() {
+	obs.close()
+}
+
+// TODO_THIS_COMMIT: decide whether this closes the notifier channel; perhaps not
+// at oll or only if it was provided...
+func (obs *notifiableObservable[V]) close() {
+	obs.observersMu.RLock()
+	observers := *obs.observers
+	obs.observersMu.RUnlock()
 
-	go unsubscribeOnDone[V](ctx, subscription)
+	for _, sub := range observers {
+		fmt.Printf("notifiableObservable#listen: unsubscribing %p\n", sub)
+		sub.Unsubscribe()
+	}
 
-	return subscription
+	obs.observersMu.Lock()
+	defer obs.observersMu.Unlock()
+	obs.observers = new([]*observer[V])
 }
 
-// listen to the notifier and notify subscribers when values are received. This
+// listen to the notifier and notify observers when values are received. This
 // function is blocking and should be run in a goroutine.
-func (obs *Observable[V]) listen(notifier <-chan V) {
-	for v := range notifier {
-		// Lock for obs.subscribers slice as it can be modified by subscribers
-		obs.mu.RLock()
-		for _, ch := range obs.subscribers {
-			ch <- v
+func (obs *notifiableObservable[V]) listen(notifier <-chan V) {
+	for notification := range notifier {
+		obs.observersMu.RLock()
+		observers := *obs.observers
+		obs.observersMu.RUnlock()
+
+		for _, sub := range observers {
+			sub.notify(notification)
 		}
-		obs.mu.RUnlock()
 	}
 
-	// Here we know that the notifier has been closed, all subscribers should be closed as well
-	obs.mu.Lock()
-	obs.closed = true
-	for _, ch := range obs.subscribers {
-		close(ch)
-		obs.subscribers = []chan V{}
-	}
-	obs.mu.Lock()
+	// Here we know that the notifier has been closed, all observers should be closed as well
+	obs.close()
 }
 
 // unsubscribeOnDone unsubscribes from the subscription when the context is.
 // It is blocking and intended to be called in a goroutine.
-func unsubscribeOnDone[V any](ctx context.Context, subscription observable.Subscription[V]) {
+func unsubscribeOnDone[V any](ctx context.Context, subscription observable.Observer[V]) {
 	if ctx != nil {
 		<-ctx.Done()
 		subscription.Unsubscribe()
 	}
 }
+
+// onUnsubscribeFactory returns a function that removes a given observer from the
+// observable's list of observers.
+func (obs *notifiableObservable[V]) onUnsubscribeFactory() UnsubscribeFunc[V] {
+	return func(toRemove *observer[V]) {
+		obs.observersMu.Lock()
+		defer obs.observersMu.Unlock()
+
+		for i, subscription := range *obs.observers {
+			if subscription == toRemove {
+				*obs.observers = append((*obs.observers)[:i], (*obs.observers)[i+1:]...)
+				break
+			}
+			obs.observersMu.Unlock()
+		}
+	}
+}
diff --git a/pkg/observable/notifiable/observable_test.go b/pkg/observable/notifiable/observable_test.go
index f3aa0bc23..d74f68135 100644
--- a/pkg/observable/notifiable/observable_test.go
+++ b/pkg/observable/notifiable/observable_test.go
@@ -18,145 +18,246 @@ const (
 	unsubscribeSleepDuration = notifyTimeout * 2
 )
 
-func TestNewNotifiableObservable(t *testing.T) {
+func TestNewObservable_NotifyObservers(t *testing.T) {
 	type test struct {
-		name     string
-		notifier chan int
+		name            string
+		notifier        chan *int
+		inputs          []int
+		expectedOutputs []int
+		setupFn         func(t test)
 	}
 
-	input := 123
-	nonEmptyBufferedNotifier := make(chan int, 1)
-	nonEmptyBufferedNotifier <- input
+	inputs := []int{123, 456, 789}
+	queuedNotifier := make(chan *int, 1)
+	nonEmptyBufferedNotifier := make(chan *int, 1)
 
 	tests := []test{
-		{name: "nil notifier", notifier: nil},
-		{name: "empty non-buffered notifier", notifier: make(chan int)},
-		{name: "empty buffered len 1 notifier", notifier: make(chan int, 1)},
-		{name: "non-empty buffered notifier", notifier: nonEmptyBufferedNotifier},
+		{
+			name:            "nil notifier",
+			notifier:        nil,
+			inputs:          inputs,
+			expectedOutputs: inputs,
+		},
+		{
+			name:            "empty non-buffered notifier",
+			notifier:        make(chan *int),
+			inputs:          inputs,
+			expectedOutputs: inputs,
+		},
+		{
+			name:            "queued non-buffered notifier",
+			notifier:        queuedNotifier,
+			inputs:          inputs[1:],
+			expectedOutputs: inputs,
+			setupFn: func(t test) {
+				go func() {
+					// blocking send
+					t.notifier <- &inputs[0]
+				}()
+			},
+		},
+		{
+			name:            "empty buffered len 1 notifier",
+			notifier:        make(chan *int, 1),
+			inputs:          inputs,
+			expectedOutputs: inputs,
+		},
+		{
+			name:            "non-empty buffered notifier",
+			notifier:        nonEmptyBufferedNotifier,
+			inputs:          inputs[1:],
+			expectedOutputs: inputs,
+			setupFn: func(t test) {
+				// non-blocking send
+				t.notifier <- &inputs[0]
+			},
+		},
 	}
 
-	for _, tt := range tests {
+	for _, tt := range tests[:] {
 		t.Run(tt.name, func(t *testing.T) {
-			ctx := context.Background()
-			notifee, notifier := notifiable.NewObservable[int](tt.notifier)
+			if tt.setupFn != nil {
+				tt.setupFn(tt)
+			}
+
+			// TECHDEBT/INCOMPLETE: test w/ & w/o context cancellation
+			//ctx := context.Background()
+			ctx, cancel := context.WithCancel(context.Background())
+			t.Cleanup(cancel)
+
+			t.Logf("notifier: %p", tt.notifier)
+			notifee, notifier := notifiable.NewObservable[*int](tt.notifier)
 			require.NotNil(t, notifee)
 			require.NotNil(t, notifier)
 
-			// construct 3 distinct subscriptions, each with its own channel
-			subscriptions := make([]observable.Subscription[int], 3)
-			for i := range subscriptions {
-				subscriptions[i] = notifee.Subscribe(ctx)
+			// construct 3 distinct observers, each with its own channel
+			observers := make([]observable.Observer[*int], 3)
+			for i := range observers {
+				observers[i] = notifee.Subscribe(ctx)
 			}
 
-			group := errgroup.Group{}
-			notifiedOrTimedOut := func(subscriptionCh <-chan int) func() error {
+			group, ctx := errgroup.WithContext(ctx)
+			notifiedOrTimedOut := func(sub observable.Observer[*int]) func() error {
+				var outputIndex int
 				return func() error {
-					// subscriptionCh should receive notified input
-					select {
-					case output := <-subscriptionCh:
-						require.Equal(t, input, output)
-					case <-time.After(notifyTimeout):
-						return fmt.Errorf("timed out waiting for subscription to be notified")
+					for {
+						select {
+						case output, ok := <-sub.Ch():
+							if !ok {
+								return nil
+							}
+
+							// observer channel should receive notified input
+							t.Logf("output: %d | %p", *output, output)
+							require.Equal(t, tt.expectedOutputs[outputIndex], *output)
+							outputIndex++
+						case <-time.After(1 * time.Second):
+							//case <-time.After(notifyTimeout):
+							return fmt.Errorf("timed out waiting for observer to be notified")
+						}
 					}
-					return nil
 				}
 			}
 
-			// ensure all subscription channels are notified
-			for _, subscription := range subscriptions {
+			// ensure all observer channels are notified
+			for _, observer := range observers {
 				// concurrently await notification or timeout to avoid blocking on
 				// empty and/or non-buffered notifiers.
-				group.Go(notifiedOrTimedOut(subscription.Ch()))
+				group.Go(notifiedOrTimedOut(observer))
 			}
 
 			// notify with test input
-			notifier <- input
+			t.Logf("sending to notifier %p", notifier)
+			for i, input := range tt.inputs[:] {
+				inputPtr := new(int)
+				*inputPtr = input
+				t.Logf("sending input ptr: %d %p", input, inputPtr)
+				notifier <- inputPtr
+				t.Logf("send input %d", i)
+			}
+			cancel()
 
 			// wait for notifee to be notified or timeout
 			err := group.Wait()
 			require.NoError(t, err)
+			t.Log("errgroup done")
 
-			// unsubscribing should close subscription channel(s)
-			for _, subscription := range subscriptions {
-				subscription.Unsubscribe()
+			// unsubscribing should close observer channel(s)
+			for i, observer := range observers {
+				observer.Unsubscribe()
+				t.Logf("unsusbscribed %d", i)
 
-				select {
-				case <-subscription.Ch():
-				default:
-					t.Fatal("subscription channel left open")
-				}
+				// must drain the channel first to ensure it is closed
+				drainCh(
+					observer.Ch(),
+					notifyTimeout,
+					func(closed bool, err error) {
+						require.NoError(t, err)
+						require.True(t, closed)
+					})
 			}
 		})
 	}
 }
 
-func TestSubscription_Unsubscribe(t *testing.T) {
+// TECHDEBT/INCOMPLETE: add coverage for multiple observers, unsubscribe from one
+// and ensure the rest are still notified.
+
+// TECHDEBT\INCOMPLETE: add coverage for active observers closing when notifier closes.
+
+func TestNewObservable_UnsubscribeObservers(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	notifee, notifier := notifiable.NewObservable[int](nil)
 	require.NotNil(t, notifee)
 	require.NotNil(t, notifier)
 
-	tests := []struct {
+	type test struct {
 		name        string
-		lifecycleFn func() observable.Subscription[int]
-	}{
+		lifecycleFn func() observable.Observer[int]
+	}
+
+	tests := []test{
 		{
 			name: "nil context",
-			lifecycleFn: func() observable.Subscription[int] {
-				subscription := notifee.Subscribe(nil)
-				subscription.Unsubscribe()
-				return subscription
+			lifecycleFn: func() observable.Observer[int] {
+				observer := notifee.Subscribe(nil)
+				observer.Unsubscribe()
+				return observer
 			},
 		},
 		{
 			name: "only unsubscribe",
-			lifecycleFn: func() observable.Subscription[int] {
-				subscription := notifee.Subscribe(ctx)
-				subscription.Unsubscribe()
-				return subscription
+			lifecycleFn: func() observable.Observer[int] {
+				observer := notifee.Subscribe(ctx)
+				observer.Unsubscribe()
+				return observer
 			},
 		},
 		{
 			name: "only cancel",
-			lifecycleFn: func() observable.Subscription[int] {
-				subscription := notifee.Subscribe(ctx)
+			lifecycleFn: func() observable.Observer[int] {
+				observer := notifee.Subscribe(ctx)
 				cancel()
-				return subscription
+				return observer
 			},
 		},
 		{
 			name: "cancel then unsubscribe",
-			lifecycleFn: func() observable.Subscription[int] {
-				subscription := notifee.Subscribe(ctx)
+			lifecycleFn: func() observable.Observer[int] {
+				observer := notifee.Subscribe(ctx)
 				cancel()
 				time.Sleep(unsubscribeSleepDuration)
-				subscription.Unsubscribe()
-				return subscription
+				observer.Unsubscribe()
+				return observer
 			},
 		},
 		{
 			name: "unsubscribe then cancel",
-			lifecycleFn: func() observable.Subscription[int] {
-				subscription := notifee.Subscribe(ctx)
-				subscription.Unsubscribe()
+			lifecycleFn: func() observable.Observer[int] {
+				observer := notifee.Subscribe(ctx)
+				observer.Unsubscribe()
 				time.Sleep(unsubscribeSleepDuration)
 				cancel()
-				return subscription
+				return observer
 			},
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			subscription := tt.lifecycleFn()
+			observer := tt.lifecycleFn()
 
 			select {
-			case value, ok := <-subscription.Ch():
+			case value, ok := <-observer.Ch():
 				require.Empty(t, value)
 				require.False(t, ok)
 			case <-time.After(notifyTimeout):
-				t.Fatal("subscription channel left open")
+				t.Fatal("observer channel left open")
 			}
 		})
 	}
 }
+
+func drainCh[V any](
+	ch <-chan V,
+	timeout time.Duration,
+	done func(closed bool, err error),
+) {
+	var err error
+drain:
+	for {
+		select {
+		case _, ok := <-ch:
+			if !ok {
+				done(true, nil)
+				break drain
+			}
+			continue
+		case <-time.After(timeout):
+			err = fmt.Errorf("timed out waiting for observer channel to close")
+		default:
+			err = fmt.Errorf("observer channel left open")
+		}
+		done(false, err)
+	}
+}
diff --git a/pkg/observable/notifiable/observer.go b/pkg/observable/notifiable/observer.go
new file mode 100644
index 000000000..7f6c9daeb
--- /dev/null
+++ b/pkg/observable/notifiable/observer.go
@@ -0,0 +1,94 @@
+package notifiable
+
+import (
+	"context"
+	"fmt"
+	"sync"
+
+	"pocket/pkg/observable"
+)
+
+// TODO_THIS_COMMIT: explain why buffer size is 1
+// observerBufferSize ...
+const observerBufferSize = 1
+
+var _ observable.Observer[any] = &observer[any]{}
+
+// observer implements the observable.Observer interface.
+type observer[V any] struct {
+	ctx        context.Context
+	observerMu *sync.RWMutex
+	observerCh chan V
+	// TODO_THIS_COMMIT: add comment
+	onUnsubscribe func(toRemove *observer[V])
+	closed        bool
+}
+
+type UnsubscribeFactory[V any] func() UnsubscribeFunc[V]
+type UnsubscribeFunc[V any] func(toRemove *observer[V])
+
+func NewSubscription[V any](
+	ctx context.Context,
+	onUnsubscribeFactory UnsubscribeFactory[V],
+) *observer[V] {
+	// Create a channel for the subscriber and append it to the observers list
+	ch := make(chan V, 1)
+	fmt.Printf("notifiableObservable#Subscribe: opening %p\n", ch)
+
+	return &observer[V]{
+		ctx:           ctx,
+		observerMu:    new(sync.RWMutex),
+		observerCh:    make(chan V, observerBufferSize),
+		onUnsubscribe: onUnsubscribeFactory(),
+	}
+}
+
+// Unsubscribe closes the subscription channel and removes the subscription from
+// the observable.
+func (obv *observer[V]) Unsubscribe() {
+	obv.observerMu.Lock()
+	defer func() {
+		obv.observerMu.Unlock()
+	}()
+
+	if obv.closed {
+		return
+	}
+
+	fmt.Printf("observer#Unsubscribe: closing %p\n", obv.observerCh)
+	close(obv.observerCh)
+	obv.closed = true
+
+	obv.onUnsubscribe(obv)
+}
+
+// Ch returns a receive-only subscription channel.
+func (obv *observer[V]) Ch() <-chan V {
+	obv.observerMu.Lock()
+	defer func() {
+		obv.observerMu.Unlock()
+	}()
+
+	return obv.observerCh
+}
+
+// TODO_CLEANUP_COMMENT: used by observable to send to subscriber  channel
+// because observer#Ch returns a receive-only channel
+func (obv *observer[V]) notify(value V) {
+	obv.observerMu.Lock()
+	ch, closed := obv.observerCh, obv.closed
+	//defer func() {
+	//	obv.observersMu.Unlock()
+	//}()
+
+	if closed {
+		obv.observerMu.Unlock()
+		return
+	}
+	obv.observerMu.Unlock()
+
+	select {
+	case ch <- value:
+	case <-obv.ctx.Done():
+	}
+}
diff --git a/pkg/observable/notifiable/observer_test.go b/pkg/observable/notifiable/observer_test.go
new file mode 100644
index 000000000..4d4ab9944
--- /dev/null
+++ b/pkg/observable/notifiable/observer_test.go
@@ -0,0 +1,34 @@
+package notifiable
+
+import (
+	"sync"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestObserver_Unsubscribe(t *testing.T) {
+	var (
+		onUnsubscribeCalled = false
+		inputCh             = make(chan int, 1)
+	)
+	sub := &observer[int]{
+		observerMu: &sync.RWMutex{},
+		// using a buffered  channel to keep the test synchronous
+		observerCh: inputCh,
+		onUnsubscribe: func(toRemove *observer[int]) {
+			onUnsubscribeCalled = true
+		},
+	}
+
+	// should initially be open
+	require.Equal(t, false, sub.closed)
+
+	inputCh <- 1
+	require.Equal(t, false, sub.closed)
+
+	sub.Unsubscribe()
+	// should be closed after `#Unsubscribe()`
+	require.Equal(t, true, sub.closed)
+	require.True(t, onUnsubscribeCalled)
+}
diff --git a/pkg/observable/notifiable/subscription.go b/pkg/observable/notifiable/subscription.go
deleted file mode 100644
index 32fce865c..000000000
--- a/pkg/observable/notifiable/subscription.go
+++ /dev/null
@@ -1,37 +0,0 @@
-package notifiable
-
-import (
-	"sync"
-
-	"pocket/pkg/observable"
-)
-
-var _ observable.Subscription[any] = &Subscription[any]{}
-
-// Subscription implements the observable.Subscription interface.
-type Subscription[V any] struct {
-	mu                   *sync.RWMutex
-	ch                   chan V
-	closed               bool
-	removeFromObservable func()
-}
-
-// Unsubscribe closes the subscription channel and removes the subscription from
-// the observable.
-func (sub *Subscription[V]) Unsubscribe() {
-	sub.mu.Lock()
-	defer sub.mu.Unlock()
-
-	if sub.closed {
-		return
-	}
-
-	close(sub.ch)
-	sub.closed = true
-	sub.removeFromObservable()
-}
-
-// Ch returns the subscription channel.
-func (sub *Subscription[V]) Ch() <-chan V {
-	return sub.ch
-}

From 6fe991dc16f27df1639fbbd9f0fed14f9a0d4a4b Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Thu, 12 Oct 2023 18:05:09 +0200
Subject: [PATCH 04/41] chore: more review improvements

---
 pkg/observable/notifiable/observable.go      | 63 +++++++++++++-------
 pkg/observable/notifiable/observable_test.go |  6 +-
 pkg/observable/notifiable/observer.go        | 11 ++--
 3 files changed, 50 insertions(+), 30 deletions(-)

diff --git a/pkg/observable/notifiable/observable.go b/pkg/observable/notifiable/observable.go
index f519f0774..089f542a8 100644
--- a/pkg/observable/notifiable/observable.go
+++ b/pkg/observable/notifiable/observable.go
@@ -10,10 +10,12 @@ import (
 
 var _ observable.Observable[any] = &notifiableObservable[any]{}
 
+type option[V any] func(obs *notifiableObservable[V])
+
 // notifiableObservable implements the observable.Observable interface and can be notified
 // via its corresponding notifier channel.
 type notifiableObservable[V any] struct {
-	notifier    <-chan V // private channel that is used to emit values to observers
+	notifier    chan V // private channel that is used to emit values to observers
 	observersMu sync.RWMutex
 	// TODO_THIS_COMMIT: update comment(s)
 	// TODO_THIS_COMMIT: consider using interface type
@@ -22,21 +24,32 @@ type notifiableObservable[V any] struct {
 
 // NewObservable creates a new observable is notified when the notifier channel
 // receives a value.
-func NewObservable[V any](notifier chan V) (observable.Observable[V], chan V) {
-	// If the caller does not provide a notifier, create a new one and return it
-	if notifier == nil {
-		notifier = make(chan V)
-	}
+// func NewObservable[V any](notifier chan V) (observable.Observable[V], chan<- V) {
+func NewObservable[V any](opts ...option[V]) (observable.Observable[V], chan<- V) {
 	observable := &notifiableObservable[V]{
-		notifier,
-		sync.RWMutex{},
-		new([]*observer[V]),
+		observersMu: sync.RWMutex{},
+		observers:   new([]*observer[V]),
+	}
+
+	for _, opt := range opts {
+		opt(observable)
+	}
+
+	// If the caller does not provide a notifier, create a new one and return it
+	if observable.notifier == nil {
+		observable.notifier = make(chan V)
 	}
 
 	// Start listening to the notifier and emit values to observers
-	go observable.listen(notifier)
+	go observable.goListen(observable.notifier)
 
-	return observable, notifier
+	return observable, observable.notifier
+}
+
+func WithNotifier[V any](notifier chan V) option[V] {
+	return func(obs *notifiableObservable[V]) {
+		obs.notifier = notifier
+	}
 }
 
 // Subscribe returns an observer which is notified when notifier receives.
@@ -46,9 +59,14 @@ func (obs *notifiableObservable[V]) Subscribe(ctx context.Context) observable.Ob
 		obs.observersMu.Unlock()
 	}()
 
-	observer := NewSubscription[V](ctx, obs.onUnsubscribeFactory)
+	observer := NewObserver[V](ctx, obs.onUnsubscribeFactory)
 
-	go unsubscribeOnDone[V](ctx, observer)
+	// caller can rely on context cancellation or call Close() to unsubscribe
+	// active observers
+	if ctx != nil {
+		// asynchronously wait for the context to close and unsubscribe
+		go goUnsubscribeOnDone[V](ctx, observer)
+	}
 	return observer
 }
 
@@ -64,7 +82,7 @@ func (obs *notifiableObservable[V]) close() {
 	obs.observersMu.RUnlock()
 
 	for _, sub := range observers {
-		fmt.Printf("notifiableObservable#listen: unsubscribing %p\n", sub)
+		fmt.Printf("notifiableObservable#goListen: unsubscribing %p\n", sub)
 		sub.Unsubscribe()
 	}
 
@@ -73,15 +91,18 @@ func (obs *notifiableObservable[V]) close() {
 	obs.observers = new([]*observer[V])
 }
 
-// listen to the notifier and notify observers when values are received. This
+// goListen to the notifier and notify observers when values are received. This
 // function is blocking and should be run in a goroutine.
-func (obs *notifiableObservable[V]) listen(notifier <-chan V) {
+func (obs *notifiableObservable[V]) goListen(notifier <-chan V) {
 	for notification := range notifier {
 		obs.observersMu.RLock()
 		observers := *obs.observers
 		obs.observersMu.RUnlock()
 
 		for _, sub := range observers {
+			// CONSIDERATION: perhaps try to avoid making this notification async
+			// as it effectively uses goroutines in memory as a buffer (with
+			// little control surface).
 			sub.notify(notification)
 		}
 	}
@@ -90,13 +111,11 @@ func (obs *notifiableObservable[V]) listen(notifier <-chan V) {
 	obs.close()
 }
 
-// unsubscribeOnDone unsubscribes from the subscription when the context is.
+// goUnsubscribeOnDone unsubscribes from the subscription when the context is.
 // It is blocking and intended to be called in a goroutine.
-func unsubscribeOnDone[V any](ctx context.Context, subscription observable.Observer[V]) {
-	if ctx != nil {
-		<-ctx.Done()
-		subscription.Unsubscribe()
-	}
+func goUnsubscribeOnDone[V any](ctx context.Context, subscription observable.Observer[V]) {
+	<-ctx.Done()
+	subscription.Unsubscribe()
 }
 
 // onUnsubscribeFactory returns a function that removes a given observer from the
diff --git a/pkg/observable/notifiable/observable_test.go b/pkg/observable/notifiable/observable_test.go
index d74f68135..75c4d163d 100644
--- a/pkg/observable/notifiable/observable_test.go
+++ b/pkg/observable/notifiable/observable_test.go
@@ -86,7 +86,9 @@ func TestNewObservable_NotifyObservers(t *testing.T) {
 			t.Cleanup(cancel)
 
 			t.Logf("notifier: %p", tt.notifier)
-			notifee, notifier := notifiable.NewObservable[*int](tt.notifier)
+			notifee, notifier := notifiable.NewObservable[*int](
+				notifiable.WithNotifier(tt.notifier),
+			)
 			require.NotNil(t, notifee)
 			require.NotNil(t, notifier)
 
@@ -167,7 +169,7 @@ func TestNewObservable_NotifyObservers(t *testing.T) {
 
 func TestNewObservable_UnsubscribeObservers(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
-	notifee, notifier := notifiable.NewObservable[int](nil)
+	notifee, notifier := notifiable.NewObservable[int]()
 	require.NotNil(t, notifee)
 	require.NotNil(t, notifier)
 
diff --git a/pkg/observable/notifiable/observer.go b/pkg/observable/notifiable/observer.go
index 7f6c9daeb..e6c50a7bd 100644
--- a/pkg/observable/notifiable/observer.go
+++ b/pkg/observable/notifiable/observer.go
@@ -27,7 +27,7 @@ type observer[V any] struct {
 type UnsubscribeFactory[V any] func() UnsubscribeFunc[V]
 type UnsubscribeFunc[V any] func(toRemove *observer[V])
 
-func NewSubscription[V any](
+func NewObserver[V any](
 	ctx context.Context,
 	onUnsubscribeFactory UnsubscribeFactory[V],
 ) *observer[V] {
@@ -77,18 +77,17 @@ func (obv *observer[V]) Ch() <-chan V {
 func (obv *observer[V]) notify(value V) {
 	obv.observerMu.Lock()
 	ch, closed := obv.observerCh, obv.closed
-	//defer func() {
-	//	obv.observersMu.Unlock()
-	//}()
+	defer obv.observerMu.Unlock()
 
 	if closed {
-		obv.observerMu.Unlock()
 		return
 	}
-	obv.observerMu.Unlock()
 
 	select {
 	case ch <- value:
 	case <-obv.ctx.Done():
+		// TECHDEBT: add a  default path which buffers values so that the sender
+		// doesn't block and other consumers can still receive.
+		// TECHDEBT: add some logic to drain the buffer at some appropriate time
 	}
 }

From 59b4b94246f5d7091d9946dc123b407750b40732 Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Fri, 13 Oct 2023 18:33:52 +0200
Subject: [PATCH 05/41] refactor: renaming

- `notifiable` pkg to `channel`
- `notifiableObservable` struct to `channelObservable`
- `observer` struct to `channelObserver`
- `notifier` vars to `producer`
- `notifee` vars to `observable` (or similar)
---
 .../{notifiable => channel}/observable.go     | 74 +++++++++----------
 .../observable_test.go                        | 72 +++++++++---------
 .../{notifiable => channel}/observer.go       | 28 +++----
 .../{notifiable => channel}/observer_test.go  |  6 +-
 4 files changed, 89 insertions(+), 91 deletions(-)
 rename pkg/observable/{notifiable => channel}/observable.go (50%)
 rename pkg/observable/{notifiable => channel}/observable_test.go (77%)
 rename pkg/observable/{notifiable => channel}/observer.go (70%)
 rename pkg/observable/{notifiable => channel}/observer_test.go (85%)

diff --git a/pkg/observable/notifiable/observable.go b/pkg/observable/channel/observable.go
similarity index 50%
rename from pkg/observable/notifiable/observable.go
rename to pkg/observable/channel/observable.go
index 089f542a8..5525edcf2 100644
--- a/pkg/observable/notifiable/observable.go
+++ b/pkg/observable/channel/observable.go
@@ -1,4 +1,4 @@
-package notifiable
+package channel
 
 import (
 	"context"
@@ -8,52 +8,50 @@ import (
 	"pocket/pkg/observable"
 )
 
-var _ observable.Observable[any] = &notifiableObservable[any]{}
+var _ observable.Observable[any] = &channelObservable[any]{}
 
-type option[V any] func(obs *notifiableObservable[V])
+type option[V any] func(obs *channelObservable[V])
 
-// notifiableObservable implements the observable.Observable interface and can be notified
-// via its corresponding notifier channel.
-type notifiableObservable[V any] struct {
-	notifier    chan V // private channel that is used to emit values to observers
+// channelObservable implements the observable.Observable interface and can be notified
+// via its corresponding producer channel.
+type channelObservable[V any] struct {
+	producer    chan V // private channel that is used to emit values to observers
 	observersMu sync.RWMutex
-	// TODO_THIS_COMMIT: update comment(s)
-	// TODO_THIS_COMMIT: consider using interface type
-	observers *[]*observer[V] // observers is a list of channels that are subscribed to the observable
+	observers   *[]*channelObserver[V] // observers is a list of channels that are subscribed to the observable
 }
 
-// NewObservable creates a new observable is notified when the notifier channel
+// NewObservable creates a new observable is notified when the producer channel
 // receives a value.
-// func NewObservable[V any](notifier chan V) (observable.Observable[V], chan<- V) {
+// func NewObservable[V any](producer chan V) (observable.Observable[V], chan<- V) {
 func NewObservable[V any](opts ...option[V]) (observable.Observable[V], chan<- V) {
-	observable := &notifiableObservable[V]{
+	obs := &channelObservable[V]{
 		observersMu: sync.RWMutex{},
-		observers:   new([]*observer[V]),
+		observers:   new([]*channelObserver[V]),
 	}
 
 	for _, opt := range opts {
-		opt(observable)
+		opt(obs)
 	}
 
-	// If the caller does not provide a notifier, create a new one and return it
-	if observable.notifier == nil {
-		observable.notifier = make(chan V)
+	// If the caller does not provide a producer, create a new one and return it
+	if obs.producer == nil {
+		obs.producer = make(chan V)
 	}
 
-	// Start listening to the notifier and emit values to observers
-	go observable.goListen(observable.notifier)
+	// Start listening to the producer and emit values to observers
+	go obs.goListen(obs.producer)
 
-	return observable, observable.notifier
+	return obs, obs.producer
 }
 
-func WithNotifier[V any](notifier chan V) option[V] {
-	return func(obs *notifiableObservable[V]) {
-		obs.notifier = notifier
+func WithProducer[V any](producer chan V) option[V] {
+	return func(obs *channelObservable[V]) {
+		obs.producer = producer
 	}
 }
 
-// Subscribe returns an observer which is notified when notifier receives.
-func (obs *notifiableObservable[V]) Subscribe(ctx context.Context) observable.Observer[V] {
+// Subscribe returns an observer which is notified when producer receives.
+func (obs *channelObservable[V]) Subscribe(ctx context.Context) observable.Observer[V] {
 	obs.observersMu.Lock()
 	defer func() {
 		obs.observersMu.Unlock()
@@ -70,31 +68,31 @@ func (obs *notifiableObservable[V]) Subscribe(ctx context.Context) observable.Ob
 	return observer
 }
 
-func (obs *notifiableObservable[V]) Close() {
+func (obs *channelObservable[V]) Close() {
 	obs.close()
 }
 
-// TODO_THIS_COMMIT: decide whether this closes the notifier channel; perhaps not
+// TODO_THIS_COMMIT: decide whether this closes the producer channel; perhaps not
 // at oll or only if it was provided...
-func (obs *notifiableObservable[V]) close() {
+func (obs *channelObservable[V]) close() {
 	obs.observersMu.RLock()
 	observers := *obs.observers
 	obs.observersMu.RUnlock()
 
 	for _, sub := range observers {
-		fmt.Printf("notifiableObservable#goListen: unsubscribing %p\n", sub)
+		fmt.Printf("channelObservable#goListen: unsubscribing %p\n", sub)
 		sub.Unsubscribe()
 	}
 
 	obs.observersMu.Lock()
 	defer obs.observersMu.Unlock()
-	obs.observers = new([]*observer[V])
+	obs.observers = new([]*channelObserver[V])
 }
 
-// goListen to the notifier and notify observers when values are received. This
+// goListen to the producer and notify observers when values are received. This
 // function is blocking and should be run in a goroutine.
-func (obs *notifiableObservable[V]) goListen(notifier <-chan V) {
-	for notification := range notifier {
+func (obs *channelObservable[V]) goListen(producer <-chan V) {
+	for notification := range producer {
 		obs.observersMu.RLock()
 		observers := *obs.observers
 		obs.observersMu.RUnlock()
@@ -107,7 +105,7 @@ func (obs *notifiableObservable[V]) goListen(notifier <-chan V) {
 		}
 	}
 
-	// Here we know that the notifier has been closed, all observers should be closed as well
+	// Here we know that the producer has been closed, all observers should be closed as well
 	obs.close()
 }
 
@@ -118,10 +116,10 @@ func goUnsubscribeOnDone[V any](ctx context.Context, subscription observable.Obs
 	subscription.Unsubscribe()
 }
 
-// onUnsubscribeFactory returns a function that removes a given observer from the
+// onUnsubscribeFactory returns a function that removes a given channelObserver from the
 // observable's list of observers.
-func (obs *notifiableObservable[V]) onUnsubscribeFactory() UnsubscribeFunc[V] {
-	return func(toRemove *observer[V]) {
+func (obs *channelObservable[V]) onUnsubscribeFactory() UnsubscribeFunc[V] {
+	return func(toRemove *channelObserver[V]) {
 		obs.observersMu.Lock()
 		defer obs.observersMu.Unlock()
 
diff --git a/pkg/observable/notifiable/observable_test.go b/pkg/observable/channel/observable_test.go
similarity index 77%
rename from pkg/observable/notifiable/observable_test.go
rename to pkg/observable/channel/observable_test.go
index 75c4d163d..281dfcf07 100644
--- a/pkg/observable/notifiable/observable_test.go
+++ b/pkg/observable/channel/observable_test.go
@@ -1,4 +1,4 @@
-package notifiable_test
+package channel_test
 
 import (
 	"context"
@@ -10,7 +10,7 @@ import (
 	"golang.org/x/sync/errgroup"
 
 	"pocket/pkg/observable"
-	"pocket/pkg/observable/notifiable"
+	"pocket/pkg/observable/channel"
 )
 
 const (
@@ -21,55 +21,55 @@ const (
 func TestNewObservable_NotifyObservers(t *testing.T) {
 	type test struct {
 		name            string
-		notifier        chan *int
+		producer        chan *int
 		inputs          []int
 		expectedOutputs []int
 		setupFn         func(t test)
 	}
 
 	inputs := []int{123, 456, 789}
-	queuedNotifier := make(chan *int, 1)
-	nonEmptyBufferedNotifier := make(chan *int, 1)
+	queuedProducer := make(chan *int, 1)
+	nonEmptyBufferedProducer := make(chan *int, 1)
 
 	tests := []test{
 		{
-			name:            "nil notifier",
-			notifier:        nil,
+			name:            "nil producer",
+			producer:        nil,
 			inputs:          inputs,
 			expectedOutputs: inputs,
 		},
 		{
-			name:            "empty non-buffered notifier",
-			notifier:        make(chan *int),
+			name:            "empty non-buffered producer",
+			producer:        make(chan *int),
 			inputs:          inputs,
 			expectedOutputs: inputs,
 		},
 		{
-			name:            "queued non-buffered notifier",
-			notifier:        queuedNotifier,
+			name:            "queued non-buffered producer",
+			producer:        queuedProducer,
 			inputs:          inputs[1:],
 			expectedOutputs: inputs,
 			setupFn: func(t test) {
 				go func() {
 					// blocking send
-					t.notifier <- &inputs[0]
+					t.producer <- &inputs[0]
 				}()
 			},
 		},
 		{
-			name:            "empty buffered len 1 notifier",
-			notifier:        make(chan *int, 1),
+			name:            "empty buffered len 1 producer",
+			producer:        make(chan *int, 1),
 			inputs:          inputs,
 			expectedOutputs: inputs,
 		},
 		{
-			name:            "non-empty buffered notifier",
-			notifier:        nonEmptyBufferedNotifier,
+			name:            "non-empty buffered producer",
+			producer:        nonEmptyBufferedProducer,
 			inputs:          inputs[1:],
 			expectedOutputs: inputs,
 			setupFn: func(t test) {
 				// non-blocking send
-				t.notifier <- &inputs[0]
+				t.producer <- &inputs[0]
 			},
 		},
 	}
@@ -85,17 +85,17 @@ func TestNewObservable_NotifyObservers(t *testing.T) {
 			ctx, cancel := context.WithCancel(context.Background())
 			t.Cleanup(cancel)
 
-			t.Logf("notifier: %p", tt.notifier)
-			notifee, notifier := notifiable.NewObservable[*int](
-				notifiable.WithNotifier(tt.notifier),
+			t.Logf("producer: %p", tt.producer)
+			testObs, producer := channel.NewObservable[*int](
+				channel.WithProducer(tt.producer),
 			)
-			require.NotNil(t, notifee)
-			require.NotNil(t, notifier)
+			require.NotNil(t, testObs)
+			require.NotNil(t, producer)
 
 			// construct 3 distinct observers, each with its own channel
 			observers := make([]observable.Observer[*int], 3)
 			for i := range observers {
-				observers[i] = notifee.Subscribe(ctx)
+				observers[i] = testObs.Subscribe(ctx)
 			}
 
 			group, ctx := errgroup.WithContext(ctx)
@@ -124,22 +124,22 @@ func TestNewObservable_NotifyObservers(t *testing.T) {
 			// ensure all observer channels are notified
 			for _, observer := range observers {
 				// concurrently await notification or timeout to avoid blocking on
-				// empty and/or non-buffered notifiers.
+				// empty and/or non-buffered producers.
 				group.Go(notifiedOrTimedOut(observer))
 			}
 
 			// notify with test input
-			t.Logf("sending to notifier %p", notifier)
+			t.Logf("sending to producer %p", producer)
 			for i, input := range tt.inputs[:] {
 				inputPtr := new(int)
 				*inputPtr = input
 				t.Logf("sending input ptr: %d %p", input, inputPtr)
-				notifier <- inputPtr
+				producer <- inputPtr
 				t.Logf("send input %d", i)
 			}
 			cancel()
 
-			// wait for notifee to be notified or timeout
+			// wait for testObs to be notified or timeout
 			err := group.Wait()
 			require.NoError(t, err)
 			t.Log("errgroup done")
@@ -165,13 +165,13 @@ func TestNewObservable_NotifyObservers(t *testing.T) {
 // TECHDEBT/INCOMPLETE: add coverage for multiple observers, unsubscribe from one
 // and ensure the rest are still notified.
 
-// TECHDEBT\INCOMPLETE: add coverage for active observers closing when notifier closes.
+// TECHDEBT\INCOMPLETE: add coverage for active observers closing when producer closes.
 
 func TestNewObservable_UnsubscribeObservers(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
-	notifee, notifier := notifiable.NewObservable[int]()
-	require.NotNil(t, notifee)
-	require.NotNil(t, notifier)
+	testObs, producer := channel.NewObservable[int]()
+	require.NotNil(t, testObs)
+	require.NotNil(t, producer)
 
 	type test struct {
 		name        string
@@ -182,7 +182,7 @@ func TestNewObservable_UnsubscribeObservers(t *testing.T) {
 		{
 			name: "nil context",
 			lifecycleFn: func() observable.Observer[int] {
-				observer := notifee.Subscribe(nil)
+				observer := testObs.Subscribe(nil)
 				observer.Unsubscribe()
 				return observer
 			},
@@ -190,7 +190,7 @@ func TestNewObservable_UnsubscribeObservers(t *testing.T) {
 		{
 			name: "only unsubscribe",
 			lifecycleFn: func() observable.Observer[int] {
-				observer := notifee.Subscribe(ctx)
+				observer := testObs.Subscribe(ctx)
 				observer.Unsubscribe()
 				return observer
 			},
@@ -198,7 +198,7 @@ func TestNewObservable_UnsubscribeObservers(t *testing.T) {
 		{
 			name: "only cancel",
 			lifecycleFn: func() observable.Observer[int] {
-				observer := notifee.Subscribe(ctx)
+				observer := testObs.Subscribe(ctx)
 				cancel()
 				return observer
 			},
@@ -206,7 +206,7 @@ func TestNewObservable_UnsubscribeObservers(t *testing.T) {
 		{
 			name: "cancel then unsubscribe",
 			lifecycleFn: func() observable.Observer[int] {
-				observer := notifee.Subscribe(ctx)
+				observer := testObs.Subscribe(ctx)
 				cancel()
 				time.Sleep(unsubscribeSleepDuration)
 				observer.Unsubscribe()
@@ -216,7 +216,7 @@ func TestNewObservable_UnsubscribeObservers(t *testing.T) {
 		{
 			name: "unsubscribe then cancel",
 			lifecycleFn: func() observable.Observer[int] {
-				observer := notifee.Subscribe(ctx)
+				observer := testObs.Subscribe(ctx)
 				observer.Unsubscribe()
 				time.Sleep(unsubscribeSleepDuration)
 				cancel()
diff --git a/pkg/observable/notifiable/observer.go b/pkg/observable/channel/observer.go
similarity index 70%
rename from pkg/observable/notifiable/observer.go
rename to pkg/observable/channel/observer.go
index e6c50a7bd..34e380cb7 100644
--- a/pkg/observable/notifiable/observer.go
+++ b/pkg/observable/channel/observer.go
@@ -1,4 +1,4 @@
-package notifiable
+package channel
 
 import (
 	"context"
@@ -12,30 +12,30 @@ import (
 // observerBufferSize ...
 const observerBufferSize = 1
 
-var _ observable.Observer[any] = &observer[any]{}
+var _ observable.Observer[any] = &channelObserver[any]{}
 
-// observer implements the observable.Observer interface.
-type observer[V any] struct {
+// channelObserver implements the observable.Observer interface.
+type channelObserver[V any] struct {
 	ctx        context.Context
 	observerMu *sync.RWMutex
 	observerCh chan V
 	// TODO_THIS_COMMIT: add comment
-	onUnsubscribe func(toRemove *observer[V])
+	onUnsubscribe func(toRemove *channelObserver[V])
 	closed        bool
 }
 
 type UnsubscribeFactory[V any] func() UnsubscribeFunc[V]
-type UnsubscribeFunc[V any] func(toRemove *observer[V])
+type UnsubscribeFunc[V any] func(toRemove *channelObserver[V])
 
 func NewObserver[V any](
 	ctx context.Context,
 	onUnsubscribeFactory UnsubscribeFactory[V],
-) *observer[V] {
+) *channelObserver[V] {
 	// Create a channel for the subscriber and append it to the observers list
 	ch := make(chan V, 1)
-	fmt.Printf("notifiableObservable#Subscribe: opening %p\n", ch)
+	fmt.Printf("channelObservable#Subscribe: opening %p\n", ch)
 
-	return &observer[V]{
+	return &channelObserver[V]{
 		ctx:           ctx,
 		observerMu:    new(sync.RWMutex),
 		observerCh:    make(chan V, observerBufferSize),
@@ -45,7 +45,7 @@ func NewObserver[V any](
 
 // Unsubscribe closes the subscription channel and removes the subscription from
 // the observable.
-func (obv *observer[V]) Unsubscribe() {
+func (obv *channelObserver[V]) Unsubscribe() {
 	obv.observerMu.Lock()
 	defer func() {
 		obv.observerMu.Unlock()
@@ -55,7 +55,7 @@ func (obv *observer[V]) Unsubscribe() {
 		return
 	}
 
-	fmt.Printf("observer#Unsubscribe: closing %p\n", obv.observerCh)
+	fmt.Printf("channelObserver#Unsubscribe: closing %p\n", obv.observerCh)
 	close(obv.observerCh)
 	obv.closed = true
 
@@ -63,7 +63,7 @@ func (obv *observer[V]) Unsubscribe() {
 }
 
 // Ch returns a receive-only subscription channel.
-func (obv *observer[V]) Ch() <-chan V {
+func (obv *channelObserver[V]) Ch() <-chan V {
 	obv.observerMu.Lock()
 	defer func() {
 		obv.observerMu.Unlock()
@@ -73,8 +73,8 @@ func (obv *observer[V]) Ch() <-chan V {
 }
 
 // TODO_CLEANUP_COMMENT: used by observable to send to subscriber  channel
-// because observer#Ch returns a receive-only channel
-func (obv *observer[V]) notify(value V) {
+// because channelObserver#Ch returns a receive-only channel
+func (obv *channelObserver[V]) notify(value V) {
 	obv.observerMu.Lock()
 	ch, closed := obv.observerCh, obv.closed
 	defer obv.observerMu.Unlock()
diff --git a/pkg/observable/notifiable/observer_test.go b/pkg/observable/channel/observer_test.go
similarity index 85%
rename from pkg/observable/notifiable/observer_test.go
rename to pkg/observable/channel/observer_test.go
index 4d4ab9944..b70e5bd9d 100644
--- a/pkg/observable/notifiable/observer_test.go
+++ b/pkg/observable/channel/observer_test.go
@@ -1,4 +1,4 @@
-package notifiable
+package channel
 
 import (
 	"sync"
@@ -12,11 +12,11 @@ func TestObserver_Unsubscribe(t *testing.T) {
 		onUnsubscribeCalled = false
 		inputCh             = make(chan int, 1)
 	)
-	sub := &observer[int]{
+	sub := &channelObserver[int]{
 		observerMu: &sync.RWMutex{},
 		// using a buffered  channel to keep the test synchronous
 		observerCh: inputCh,
-		onUnsubscribe: func(toRemove *observer[int]) {
+		onUnsubscribe: func(toRemove *channelObserver[int]) {
 			onUnsubscribeCalled = true
 		},
 	}

From 84ed42d23747f6520d3addaea393fc0d1461214c Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Fri, 13 Oct 2023 18:49:15 +0200
Subject: [PATCH 06/41] chore: update comments

---
 pkg/observable/channel/observable.go | 26 ++++++++++++++++++--------
 pkg/observable/channel/observer.go   | 15 +++++++++++----
 pkg/observable/interface.go          |  5 +++++
 3 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/pkg/observable/channel/observable.go b/pkg/observable/channel/observable.go
index 5525edcf2..5f0cd82fb 100644
--- a/pkg/observable/channel/observable.go
+++ b/pkg/observable/channel/observable.go
@@ -10,20 +10,27 @@ import (
 
 var _ observable.Observable[any] = &channelObservable[any]{}
 
+// option is a function which receives and can modify the channelObservable state.
 type option[V any] func(obs *channelObservable[V])
 
 // channelObservable implements the observable.Observable interface and can be notified
 // via its corresponding producer channel.
 type channelObservable[V any] struct {
-	producer    chan V // private channel that is used to emit values to observers
+	// producer is an observable-wide channel that is used to receive values
+	// which are subsequently re-sent to observers.
+	producer chan V
+	// observersMu protects observers from concurrent access/updates
 	observersMu sync.RWMutex
-	observers   *[]*channelObserver[V] // observers is a list of channels that are subscribed to the observable
+	// observers is a list of channelObservers that will be notified with producer
+	// receives a value.
+	observers *[]*channelObserver[V]
 }
 
 // NewObservable creates a new observable is notified when the producer channel
 // receives a value.
 // func NewObservable[V any](producer chan V) (observable.Observable[V], chan<- V) {
 func NewObservable[V any](opts ...option[V]) (observable.Observable[V], chan<- V) {
+	// initialize an observer that publishes messages from 1 producer to N observers
 	obs := &channelObservable[V]{
 		observersMu: sync.RWMutex{},
 		observers:   new([]*channelObserver[V]),
@@ -33,24 +40,27 @@ func NewObservable[V any](opts ...option[V]) (observable.Observable[V], chan<- V
 		opt(obs)
 	}
 
-	// If the caller does not provide a producer, create a new one and return it
+	// if the caller does not provide a producer, create a new one and return it
 	if obs.producer == nil {
 		obs.producer = make(chan V)
 	}
 
-	// Start listening to the producer and emit values to observers
+	// start listening to the producer and emit values to observers
 	go obs.goListen(obs.producer)
 
 	return obs, obs.producer
 }
 
+// WithProducer returns an option function sets the given producer in an observable
+// when passed to NewObservable().
 func WithProducer[V any](producer chan V) option[V] {
 	return func(obs *channelObservable[V]) {
 		obs.producer = producer
 	}
 }
 
-// Subscribe returns an observer which is notified when producer receives.
+// Subscribe returns an observer which is notified when the producer channel
+// receives a value.
 func (obs *channelObservable[V]) Subscribe(ctx context.Context) observable.Observer[V] {
 	obs.observersMu.Lock()
 	defer func() {
@@ -98,9 +108,9 @@ func (obs *channelObservable[V]) goListen(producer <-chan V) {
 		obs.observersMu.RUnlock()
 
 		for _, sub := range observers {
-			// CONSIDERATION: perhaps try to avoid making this notification async
-			// as it effectively uses goroutines in memory as a buffer (with
-			// little control surface).
+			// CONSIDERATION: perhaps continue trying to avoid making this
+			// notification async as it would effectively use goroutines
+			// in memory as a buffer (with little control surface).
 			sub.notify(notification)
 		}
 	}
diff --git a/pkg/observable/channel/observer.go b/pkg/observable/channel/observer.go
index 34e380cb7..9f5f427d3 100644
--- a/pkg/observable/channel/observer.go
+++ b/pkg/observable/channel/observer.go
@@ -16,12 +16,19 @@ var _ observable.Observer[any] = &channelObserver[any]{}
 
 // channelObserver implements the observable.Observer interface.
 type channelObserver[V any] struct {
-	ctx        context.Context
+	ctx context.Context
+	// onUnsubscribe is called in Observer#Unsubscribe, removing the respective
+	// observer from observers in a concurrency-safe manner.
+	onUnsubscribe func(toRemove *channelObserver[V])
+	// observerMu protects the observerCh and closed fields.
 	observerMu *sync.RWMutex
+	// observerCh is the channel that is used to emit values to the observer.
+	// I.e. on the "N" side of the 1:N relationship between observable and
+	// observer.
 	observerCh chan V
-	// TODO_THIS_COMMIT: add comment
-	onUnsubscribe func(toRemove *channelObserver[V])
-	closed        bool
+	// closed indicates whether the observer has been closed. It's set in
+	// unsubscribe; closed observers can't be reused.
+	closed bool
 }
 
 type UnsubscribeFactory[V any] func() UnsubscribeFunc[V]
diff --git a/pkg/observable/interface.go b/pkg/observable/interface.go
index 8e53e914d..3b224c889 100644
--- a/pkg/observable/interface.go
+++ b/pkg/observable/interface.go
@@ -2,6 +2,11 @@ package observable
 
 import "context"
 
+// NOTE: We explicitly decided to write a small and custom notifications package
+// to keep logic simple and minimal. If the needs & requirements of this library ever
+// grow, other packages (e.g. https://github.com/ReactiveX/RxGo) can be considered.
+// (see: https://github.com/ReactiveX/RxGo/pull/377)
+
 // Observable is a generic interface that allows multiple subscribers to be
 // notified of new values asynchronously.
 type Observable[V any] interface {

From da475a4dd8a6b398400776ed9c238ed93af7a3cb Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Fri, 13 Oct 2023 19:05:36 +0200
Subject: [PATCH 07/41] refactor: simplify drainCh test helper

---
 pkg/observable/channel/observable_test.go | 27 ++++++-----------------
 1 file changed, 7 insertions(+), 20 deletions(-)

diff --git a/pkg/observable/channel/observable_test.go b/pkg/observable/channel/observable_test.go
index 281dfcf07..334f179cf 100644
--- a/pkg/observable/channel/observable_test.go
+++ b/pkg/observable/channel/observable_test.go
@@ -150,13 +150,9 @@ func TestNewObservable_NotifyObservers(t *testing.T) {
 				t.Logf("unsusbscribed %d", i)
 
 				// must drain the channel first to ensure it is closed
-				drainCh(
-					observer.Ch(),
-					notifyTimeout,
-					func(closed bool, err error) {
-						require.NoError(t, err)
-						require.True(t, closed)
-					})
+				closed, err := drainCh(observer.Ch())
+				require.NoError(t, err)
+				require.True(t, closed)
 			}
 		})
 	}
@@ -240,26 +236,17 @@ func TestNewObservable_UnsubscribeObservers(t *testing.T) {
 	}
 }
 
-func drainCh[V any](
-	ch <-chan V,
-	timeout time.Duration,
-	done func(closed bool, err error),
-) {
-	var err error
-drain:
+func drainCh[V any](ch <-chan V) (closed bool, err error) {
 	for {
 		select {
 		case _, ok := <-ch:
 			if !ok {
-				done(true, nil)
-				break drain
+				return true, nil
+				return
 			}
 			continue
-		case <-time.After(timeout):
-			err = fmt.Errorf("timed out waiting for observer channel to close")
 		default:
-			err = fmt.Errorf("observer channel left open")
+			return false, fmt.Errorf("observer channel left open")
 		}
-		done(false, err)
 	}
 }

From 2fae1bf05f6f05579612299de12f8501fa8a813d Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Fri, 13 Oct 2023 19:05:50 +0200
Subject: [PATCH 08/41] test: fix timeout

---
 pkg/observable/channel/observable_test.go | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/pkg/observable/channel/observable_test.go b/pkg/observable/channel/observable_test.go
index 334f179cf..7be0bb969 100644
--- a/pkg/observable/channel/observable_test.go
+++ b/pkg/observable/channel/observable_test.go
@@ -113,8 +113,7 @@ func TestNewObservable_NotifyObservers(t *testing.T) {
 							t.Logf("output: %d | %p", *output, output)
 							require.Equal(t, tt.expectedOutputs[outputIndex], *output)
 							outputIndex++
-						case <-time.After(1 * time.Second):
-							//case <-time.After(notifyTimeout):
+						case <-time.After(notifyTimeout):
 							return fmt.Errorf("timed out waiting for observer to be notified")
 						}
 					}

From 5a4f2440d3a4d5ec18661f2f5bed0c360d6ac1dd Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Fri, 13 Oct 2023 19:11:04 +0200
Subject: [PATCH 09/41] test: rename observable test functions

---
 pkg/observable/channel/observable_test.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/pkg/observable/channel/observable_test.go b/pkg/observable/channel/observable_test.go
index 7be0bb969..979d23d1a 100644
--- a/pkg/observable/channel/observable_test.go
+++ b/pkg/observable/channel/observable_test.go
@@ -18,7 +18,7 @@ const (
 	unsubscribeSleepDuration = notifyTimeout * 2
 )
 
-func TestNewObservable_NotifyObservers(t *testing.T) {
+func TestChannelObservable_NotifyObservers(t *testing.T) {
 	type test struct {
 		name            string
 		producer        chan *int
@@ -162,7 +162,7 @@ func TestNewObservable_NotifyObservers(t *testing.T) {
 
 // TECHDEBT\INCOMPLETE: add coverage for active observers closing when producer closes.
 
-func TestNewObservable_UnsubscribeObservers(t *testing.T) {
+func TestChannelObservable_UnsubscribeObservers(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	testObs, producer := channel.NewObservable[int]()
 	require.NotNil(t, testObs)

From c1d92be48a5c46aff8f82eed81e5afd25f9edfa5 Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Fri, 13 Oct 2023 19:11:08 +0200
Subject: [PATCH 10/41] test: add test TODOs

---
 pkg/observable/channel/observable_test.go | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/pkg/observable/channel/observable_test.go b/pkg/observable/channel/observable_test.go
index 979d23d1a..b822c1fde 100644
--- a/pkg/observable/channel/observable_test.go
+++ b/pkg/observable/channel/observable_test.go
@@ -159,8 +159,14 @@ func TestChannelObservable_NotifyObservers(t *testing.T) {
 
 // TECHDEBT/INCOMPLETE: add coverage for multiple observers, unsubscribe from one
 // and ensure the rest are still notified.
+func TestChannelObservable_IndependentObservers(t *testing.T) {
+	t.Skip("add coverage: unsubscribing one observer should not impact the rest")
+}
 
 // TECHDEBT\INCOMPLETE: add coverage for active observers closing when producer closes.
+func TestChannelObservable_ObserversCloseOnProducerClose(t *testing.T) {
+	t.Skip("add coverage: all observers should close when producer closes")
+}
 
 func TestChannelObservable_UnsubscribeObservers(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())

From bf8bc8a1a11bdc6dee512c72b9f29898df40d68d Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Fri, 13 Oct 2023 19:32:46 +0200
Subject: [PATCH 11/41] chore: update comments

---
 pkg/observable/channel/observable.go | 4 ++--
 pkg/observable/channel/observer.go   | 6 ++++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/pkg/observable/channel/observable.go b/pkg/observable/channel/observable.go
index 5f0cd82fb..3e263a111 100644
--- a/pkg/observable/channel/observable.go
+++ b/pkg/observable/channel/observable.go
@@ -82,8 +82,8 @@ func (obs *channelObservable[V]) Close() {
 	obs.close()
 }
 
-// TODO_THIS_COMMIT: decide whether this closes the producer channel; perhaps not
-// at oll or only if it was provided...
+// CONSIDERATION: decide whether this should close the producer channel; perhaps
+// only if it was provided.
 func (obs *channelObservable[V]) close() {
 	obs.observersMu.RLock()
 	observers := *obs.observers
diff --git a/pkg/observable/channel/observer.go b/pkg/observable/channel/observer.go
index 9f5f427d3..8cb64e950 100644
--- a/pkg/observable/channel/observer.go
+++ b/pkg/observable/channel/observer.go
@@ -8,8 +8,10 @@ import (
 	"pocket/pkg/observable"
 )
 
-// TODO_THIS_COMMIT: explain why buffer size is 1
-// observerBufferSize ...
+// DISCUSS: what should this be? should it be configurable? It seems to be most
+// relevant in the context of the behavior of the observable when it has multiple
+// observers which consume at different rates.
+// observerBufferSize is the buffer size of a channelObserver's channel.
 const observerBufferSize = 1
 
 var _ observable.Observer[any] = &channelObserver[any]{}

From 8e17e83764c0508c1c5a5aa837d3b18198d953af Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Sat, 14 Oct 2023 01:04:02 +0200
Subject: [PATCH 12/41] refactor: simplify observable & observer

---
 pkg/observable/channel/observable.go | 80 +++++++++++++++-------------
 pkg/observable/channel/observer.go   | 37 +++++++------
 2 files changed, 60 insertions(+), 57 deletions(-)

diff --git a/pkg/observable/channel/observable.go b/pkg/observable/channel/observable.go
index 3e263a111..85122593d 100644
--- a/pkg/observable/channel/observable.go
+++ b/pkg/observable/channel/observable.go
@@ -20,10 +20,10 @@ type channelObservable[V any] struct {
 	// which are subsequently re-sent to observers.
 	producer chan V
 	// observersMu protects observers from concurrent access/updates
-	observersMu sync.RWMutex
-	// observers is a list of channelObservers that will be notified with producer
-	// receives a value.
-	observers *[]*channelObserver[V]
+	observersMu *sync.RWMutex
+	// observers is a list of channelObservers that will be notified when producer
+	// receives a value. It is a pointer because ...
+	observers []*channelObserver[V]
 }
 
 // NewObservable creates a new observable is notified when the producer channel
@@ -32,8 +32,8 @@ type channelObservable[V any] struct {
 func NewObservable[V any](opts ...option[V]) (observable.Observable[V], chan<- V) {
 	// initialize an observer that publishes messages from 1 producer to N observers
 	obs := &channelObservable[V]{
-		observersMu: sync.RWMutex{},
-		observers:   new([]*channelObserver[V]),
+		observersMu: &sync.RWMutex{},
+		observers:   []*channelObserver[V]{},
 	}
 
 	for _, opt := range opts {
@@ -61,13 +61,12 @@ func WithProducer[V any](producer chan V) option[V] {
 
 // Subscribe returns an observer which is notified when the producer channel
 // receives a value.
-func (obs *channelObservable[V]) Subscribe(ctx context.Context) observable.Observer[V] {
-	obs.observersMu.Lock()
-	defer func() {
-		obs.observersMu.Unlock()
-	}()
+func (obsvbl *channelObservable[V]) Subscribe(ctx context.Context) observable.Observer[V] {
+	obsvbl.observersMu.Lock()
+	defer obsvbl.observersMu.Unlock()
 
-	observer := NewObserver[V](ctx, obs.onUnsubscribeFactory)
+	observer := NewObserver[V](ctx, obsvbl.onUnsubscribeFactory)
+	obsvbl.observers = append(obsvbl.observers, observer)
 
 	// caller can rely on context cancellation or call Close() to unsubscribe
 	// active observers
@@ -78,45 +77,50 @@ func (obs *channelObservable[V]) Subscribe(ctx context.Context) observable.Obser
 	return observer
 }
 
-func (obs *channelObservable[V]) Close() {
-	obs.close()
+func (obsvbl *channelObservable[V]) Close() {
+	obsvbl.close()
 }
 
 // CONSIDERATION: decide whether this should close the producer channel; perhaps
 // only if it was provided.
-func (obs *channelObservable[V]) close() {
-	obs.observersMu.RLock()
-	observers := *obs.observers
-	obs.observersMu.RUnlock()
-
-	for _, sub := range observers {
-		fmt.Printf("channelObservable#goListen: unsubscribing %p\n", sub)
-		sub.Unsubscribe()
+func (obsvbl *channelObservable[V]) close() {
+	obsvbl.observersMu.Lock()
+	defer obsvbl.observersMu.Unlock()
+
+	observers := obsvbl.observers
+
+	for _, obsvr := range observers {
+		fmt.Printf("channelObservable#goListen: unsubscribing %p\n", obsvr)
+		obsvr.Unsubscribe()
 	}
 
-	obs.observersMu.Lock()
-	defer obs.observersMu.Unlock()
-	obs.observers = new([]*channelObserver[V])
+	// clear observers
+	obsvbl.observers = []*channelObserver[V]{}
 }
 
 // goListen to the producer and notify observers when values are received. This
 // function is blocking and should be run in a goroutine.
-func (obs *channelObservable[V]) goListen(producer <-chan V) {
+func (obsvbl *channelObservable[V]) goListen(producer <-chan V) {
 	for notification := range producer {
-		obs.observersMu.RLock()
-		observers := *obs.observers
-		obs.observersMu.RUnlock()
+		// copy observers to avoid holding the lock while notifying
+		obsvbl.observersMu.RLock()
+		observers := make([]*channelObserver[V], len(obsvbl.observers))
+		for i, obsvr := range obsvbl.observers {
+			observers[i] = obsvr
+		}
+		obsvbl.observersMu.RUnlock()
 
-		for _, sub := range observers {
+		// notify observers
+		for _, obsvr := range observers {
 			// CONSIDERATION: perhaps continue trying to avoid making this
 			// notification async as it would effectively use goroutines
 			// in memory as a buffer (with little control surface).
-			sub.notify(notification)
+			obsvr.notify(notification)
 		}
 	}
 
 	// Here we know that the producer has been closed, all observers should be closed as well
-	obs.close()
+	obsvbl.close()
 }
 
 // goUnsubscribeOnDone unsubscribes from the subscription when the context is.
@@ -128,17 +132,17 @@ func goUnsubscribeOnDone[V any](ctx context.Context, subscription observable.Obs
 
 // onUnsubscribeFactory returns a function that removes a given channelObserver from the
 // observable's list of observers.
-func (obs *channelObservable[V]) onUnsubscribeFactory() UnsubscribeFunc[V] {
+func (obsvbl *channelObservable[V]) onUnsubscribeFactory() UnsubscribeFunc[V] {
 	return func(toRemove *channelObserver[V]) {
-		obs.observersMu.Lock()
-		defer obs.observersMu.Unlock()
+		obsvbl.observersMu.Lock()
+		defer obsvbl.observersMu.Unlock()
+		observers := obsvbl.observers
 
-		for i, subscription := range *obs.observers {
+		for i, subscription := range observers {
 			if subscription == toRemove {
-				*obs.observers = append((*obs.observers)[:i], (*obs.observers)[i+1:]...)
+				obsvbl.observers = append((obsvbl.observers)[:i], (obsvbl.observers)[i+1:]...)
 				break
 			}
-			obs.observersMu.Unlock()
 		}
 	}
 }
diff --git a/pkg/observable/channel/observer.go b/pkg/observable/channel/observer.go
index 8cb64e950..86f202fa2 100644
--- a/pkg/observable/channel/observer.go
+++ b/pkg/observable/channel/observer.go
@@ -54,47 +54,46 @@ func NewObserver[V any](
 
 // Unsubscribe closes the subscription channel and removes the subscription from
 // the observable.
-func (obv *channelObserver[V]) Unsubscribe() {
-	obv.observerMu.Lock()
+func (obsvr *channelObserver[V]) Unsubscribe() {
+	obsvr.observerMu.Lock()
 	defer func() {
-		obv.observerMu.Unlock()
+		obsvr.observerMu.Unlock()
 	}()
 
-	if obv.closed {
+	if obsvr.closed {
 		return
 	}
 
-	fmt.Printf("channelObserver#Unsubscribe: closing %p\n", obv.observerCh)
-	close(obv.observerCh)
-	obv.closed = true
+	fmt.Printf("channelObserver#Unsubscribe: closing %p\n", obsvr.observerCh)
+	close(obsvr.observerCh)
+	obsvr.closed = true
 
-	obv.onUnsubscribe(obv)
+	obsvr.onUnsubscribe(obsvr)
 }
 
 // Ch returns a receive-only subscription channel.
-func (obv *channelObserver[V]) Ch() <-chan V {
-	obv.observerMu.Lock()
+func (obsvr *channelObserver[V]) Ch() <-chan V {
+	obsvr.observerMu.Lock()
 	defer func() {
-		obv.observerMu.Unlock()
+		obsvr.observerMu.Unlock()
 	}()
 
-	return obv.observerCh
+	return obsvr.observerCh
 }
 
 // TODO_CLEANUP_COMMENT: used by observable to send to subscriber  channel
 // because channelObserver#Ch returns a receive-only channel
-func (obv *channelObserver[V]) notify(value V) {
-	obv.observerMu.Lock()
-	ch, closed := obv.observerCh, obv.closed
-	defer obv.observerMu.Unlock()
+func (obsvr *channelObserver[V]) notify(value V) {
+	obsvr.observerMu.Lock()
+	defer obsvr.observerMu.Unlock()
 
-	if closed {
+	if obsvr.closed {
 		return
 	}
 
 	select {
-	case ch <- value:
-	case <-obv.ctx.Done():
+	case obsvr.observerCh <- value:
+	case <-obsvr.ctx.Done():
 		// TECHDEBT: add a  default path which buffers values so that the sender
 		// doesn't block and other consumers can still receive.
 		// TECHDEBT: add some logic to drain the buffer at some appropriate time

From 83d4a2049cb63e413e61d879572ccecebe46f69c Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Sat, 14 Oct 2023 01:04:33 +0200
Subject: [PATCH 13/41] test: fix & add observable tests

---
 pkg/observable/channel/observable_test.go | 282 +++++++++++++++++-----
 pkg/observable/channel/observer_test.go   |  10 +-
 2 files changed, 232 insertions(+), 60 deletions(-)

diff --git a/pkg/observable/channel/observable_test.go b/pkg/observable/channel/observable_test.go
index b822c1fde..186540fb4 100644
--- a/pkg/observable/channel/observable_test.go
+++ b/pkg/observable/channel/observable_test.go
@@ -3,9 +3,11 @@ package channel_test
 import (
 	"context"
 	"fmt"
+	"sync"
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/sync/errgroup"
 
@@ -14,7 +16,8 @@ import (
 )
 
 const (
-	notifyTimeout            = 100 * time.Millisecond
+	productionDelay          = 10 * time.Millisecond
+	notifyTimeout            = productionDelay * 4
 	unsubscribeSleepDuration = notifyTimeout * 2
 )
 
@@ -28,8 +31,8 @@ func TestChannelObservable_NotifyObservers(t *testing.T) {
 	}
 
 	inputs := []int{123, 456, 789}
-	queuedProducer := make(chan *int, 1)
-	nonEmptyBufferedProducer := make(chan *int, 1)
+	fullBlockingProducer := make(chan *int)
+	fullBufferedProducer := make(chan *int, 1)
 
 	tests := []test{
 		{
@@ -45,8 +48,8 @@ func TestChannelObservable_NotifyObservers(t *testing.T) {
 			expectedOutputs: inputs,
 		},
 		{
-			name:            "queued non-buffered producer",
-			producer:        queuedProducer,
+			name:            "full non-buffered producer",
+			producer:        fullBlockingProducer,
 			inputs:          inputs[1:],
 			expectedOutputs: inputs,
 			setupFn: func(t test) {
@@ -63,8 +66,8 @@ func TestChannelObservable_NotifyObservers(t *testing.T) {
 			expectedOutputs: inputs,
 		},
 		{
-			name:            "non-empty buffered producer",
-			producer:        nonEmptyBufferedProducer,
+			name:            "full buffered len 1 producer",
+			producer:        fullBufferedProducer,
 			inputs:          inputs[1:],
 			expectedOutputs: inputs,
 			setupFn: func(t test) {
@@ -74,7 +77,7 @@ func TestChannelObservable_NotifyObservers(t *testing.T) {
 		},
 	}
 
-	for _, tt := range tests[:] {
+	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			if tt.setupFn != nil {
 				tt.setupFn(tt)
@@ -86,64 +89,70 @@ func TestChannelObservable_NotifyObservers(t *testing.T) {
 			t.Cleanup(cancel)
 
 			t.Logf("producer: %p", tt.producer)
-			testObs, producer := channel.NewObservable[*int](
+			obsvbl, producer := channel.NewObservable[*int](
 				channel.WithProducer(tt.producer),
 			)
-			require.NotNil(t, testObs)
+			require.NotNil(t, obsvbl)
 			require.NotNil(t, producer)
 
 			// construct 3 distinct observers, each with its own channel
-			observers := make([]observable.Observer[*int], 3)
+			observers := make([]observable.Observer[*int], 1)
 			for i := range observers {
-				observers[i] = testObs.Subscribe(ctx)
+				observers[i] = obsvbl.Subscribe(ctx)
 			}
 
 			group, ctx := errgroup.WithContext(ctx)
-			notifiedOrTimedOut := func(sub observable.Observer[*int]) func() error {
-				var outputIndex int
-				return func() error {
-					for {
-						select {
-						case output, ok := <-sub.Ch():
-							if !ok {
-								return nil
-							}
-
-							// observer channel should receive notified input
-							t.Logf("output: %d | %p", *output, output)
-							require.Equal(t, tt.expectedOutputs[outputIndex], *output)
-							outputIndex++
-						case <-time.After(notifyTimeout):
-							return fmt.Errorf("timed out waiting for observer to be notified")
-						}
+
+			// ensure all obsvr channels are notified
+			for obsvrIdx, obsvr := range observers {
+				next := func(outputIndex int, output *int) error {
+					// obsvr channel should receive notified input
+					t.Logf("output: %d | %p", *output, output)
+					if !assert.Equalf(
+						t, tt.expectedOutputs[outputIndex],
+						*output,
+						"obsvr Idx: %d", obsvrIdx,
+					) {
+						return fmt.Errorf("unexpected output")
 					}
+					return nil
+				}
+
+				done := func(outputs []*int) error {
+					if !assert.Equalf(
+						t, len(tt.expectedOutputs),
+						len(outputs),
+						"obsvr addr: %p", obsvr,
+					) {
+						return fmt.Errorf("unexpected number of outputs")
+					}
+					return nil
 				}
-			}
 
-			// ensure all observer channels are notified
-			for _, observer := range observers {
 				// concurrently await notification or timeout to avoid blocking on
 				// empty and/or non-buffered producers.
-				group.Go(notifiedOrTimedOut(observer))
+				group.Go(goNotifiedOrTimedOutFactory(obsvr, next, done))
 			}
 
 			// notify with test input
 			t.Logf("sending to producer %p", producer)
-			for i, input := range tt.inputs[:] {
+			for i, input := range tt.inputs {
 				inputPtr := new(int)
 				*inputPtr = input
 				t.Logf("sending input ptr: %d %p", input, inputPtr)
+				time.Sleep(productionDelay)
 				producer <- inputPtr
+				time.Sleep(productionDelay)
 				t.Logf("send input %d", i)
 			}
 			cancel()
 
-			// wait for testObs to be notified or timeout
+			// wait for obsvbl to be notified or timeout
 			err := group.Wait()
 			require.NoError(t, err)
 			t.Log("errgroup done")
 
-			// unsubscribing should close observer channel(s)
+			// unsubscribing should close obsvr channel(s)
 			for i, observer := range observers {
 				observer.Unsubscribe()
 				t.Logf("unsusbscribed %d", i)
@@ -157,21 +166,10 @@ func TestChannelObservable_NotifyObservers(t *testing.T) {
 	}
 }
 
-// TECHDEBT/INCOMPLETE: add coverage for multiple observers, unsubscribe from one
-// and ensure the rest are still notified.
-func TestChannelObservable_IndependentObservers(t *testing.T) {
-	t.Skip("add coverage: unsubscribing one observer should not impact the rest")
-}
-
-// TECHDEBT\INCOMPLETE: add coverage for active observers closing when producer closes.
-func TestChannelObservable_ObserversCloseOnProducerClose(t *testing.T) {
-	t.Skip("add coverage: all observers should close when producer closes")
-}
-
 func TestChannelObservable_UnsubscribeObservers(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
-	testObs, producer := channel.NewObservable[int]()
-	require.NotNil(t, testObs)
+	obsvbl, producer := channel.NewObservable[int]()
+	require.NotNil(t, obsvbl)
 	require.NotNil(t, producer)
 
 	type test struct {
@@ -183,7 +181,7 @@ func TestChannelObservable_UnsubscribeObservers(t *testing.T) {
 		{
 			name: "nil context",
 			lifecycleFn: func() observable.Observer[int] {
-				observer := testObs.Subscribe(nil)
+				observer := obsvbl.Subscribe(nil)
 				observer.Unsubscribe()
 				return observer
 			},
@@ -191,7 +189,7 @@ func TestChannelObservable_UnsubscribeObservers(t *testing.T) {
 		{
 			name: "only unsubscribe",
 			lifecycleFn: func() observable.Observer[int] {
-				observer := testObs.Subscribe(ctx)
+				observer := obsvbl.Subscribe(ctx)
 				observer.Unsubscribe()
 				return observer
 			},
@@ -199,7 +197,7 @@ func TestChannelObservable_UnsubscribeObservers(t *testing.T) {
 		{
 			name: "only cancel",
 			lifecycleFn: func() observable.Observer[int] {
-				observer := testObs.Subscribe(ctx)
+				observer := obsvbl.Subscribe(ctx)
 				cancel()
 				return observer
 			},
@@ -207,7 +205,7 @@ func TestChannelObservable_UnsubscribeObservers(t *testing.T) {
 		{
 			name: "cancel then unsubscribe",
 			lifecycleFn: func() observable.Observer[int] {
-				observer := testObs.Subscribe(ctx)
+				observer := obsvbl.Subscribe(ctx)
 				cancel()
 				time.Sleep(unsubscribeSleepDuration)
 				observer.Unsubscribe()
@@ -217,7 +215,7 @@ func TestChannelObservable_UnsubscribeObservers(t *testing.T) {
 		{
 			name: "unsubscribe then cancel",
 			lifecycleFn: func() observable.Observer[int] {
-				observer := testObs.Subscribe(ctx)
+				observer := obsvbl.Subscribe(ctx)
 				observer.Unsubscribe()
 				time.Sleep(unsubscribeSleepDuration)
 				cancel()
@@ -247,7 +245,6 @@ func drainCh[V any](ch <-chan V) (closed bool, err error) {
 		case _, ok := <-ch:
 			if !ok {
 				return true, nil
-				return
 			}
 			continue
 		default:
@@ -255,3 +252,178 @@ func drainCh[V any](ch <-chan V) (closed bool, err error) {
 		}
 	}
 }
+
+func TestChannelObservable_ConcurrentSubUnSub(t *testing.T) {
+	t.Skip("add coverage: subscribing and unsubscribing concurrently should not race")
+}
+
+// TECHDEBT/INCOMPLETE: add coverage for multiple observers, unsubscribe from one
+// and ensure the rest are still notified.
+// TODO_THIS_COMMIT: consider renaming, also has to do with sequential notificaions
+func TestChannelObservable_ObserversUnsubscribeIndependently(t *testing.T) {
+	//t.Skip("add coverage: unsubscribing one observer should not impact the rest")
+
+	observations := new([]*observation[int])
+	expectedNotifications := [][]int{
+		{123, 456, 789},
+		{456, 789, 987},
+		{789, 987, 654},
+		{987, 654, 321},
+	}
+
+	//type observation struct {
+	//	observerIndex int
+	//	notifications []int
+	//}
+
+	obsvbl, producer := channel.NewObservable[int]()
+	require.NotNil(t, obsvbl)
+	require.NotNil(t, producer)
+	produceWithDelay := syncSendWithDelayFactory(producer, productionDelay)
+
+	ctx, cancel := context.WithCancel(context.Background())
+	t.Cleanup(cancel)
+
+	observation0 := newObservation(ctx, obsvbl)
+	*observations = append(*observations, observation0)
+	go goReceiveNotifications(observation0)
+	produceWithDelay(123)
+
+	observation1 := newObservation(ctx, obsvbl)
+	*observations = append(*observations, observation1)
+	go goReceiveNotifications(observation1)
+	produceWithDelay(456)
+
+	observation2 := newObservation(ctx, obsvbl)
+	*observations = append(*observations, observation2)
+	go goReceiveNotifications(observation2)
+	produceWithDelay(789)
+
+	observation3 := newObservation(ctx, obsvbl)
+	*observations = append(*observations, observation3)
+	go goReceiveNotifications(observation3)
+
+	observation0.Unsubscribe()
+	produceWithDelay(987)
+
+	observation1.Unsubscribe()
+	produceWithDelay(654)
+
+	observation2.Unsubscribe()
+	produceWithDelay(321)
+
+	observation3.Unsubscribe()
+
+	for obsnIdx, obsrvn := range *observations {
+		t.Run(fmt.Sprintf("observation%d", obsnIdx), func(t *testing.T) {
+			msg := "observer %d channel left open"
+			select {
+			case _, ok := <-obsrvn.Ch():
+				require.Falsef(t, ok, msg, obsnIdx)
+			default:
+				t.Fatalf(msg, obsnIdx)
+			}
+
+			obsrvn.Lock()
+			defer obsrvn.Unlock()
+			require.Equalf(
+				t, len(expectedNotifications[obsnIdx]),
+				len(*obsrvn.Notifications),
+				"observation index: %d, expected: %+v, actual: %+v",
+				obsnIdx, expectedNotifications[obsnIdx], *obsrvn.Notifications,
+			)
+			for notificationIdx, expected := range expectedNotifications[obsnIdx] {
+				require.Equalf(
+					t, expected,
+					(*obsrvn.Notifications)[notificationIdx],
+					"allExpected: %+v, allActual: %+v",
+					expectedNotifications[obsnIdx], *obsrvn.Notifications,
+				)
+			}
+		})
+	}
+}
+
+// TECHDEBT/INCOMPLETE: add coverage for active observers closing when producer closes.
+func TestChannelObservable_ObserversCloseOnProducerClose(t *testing.T) {
+	t.Skip("add coverage: all observers should close when producer closes")
+}
+
+// TECHDEBT/INCOMPLETE: add coverage for observers (un)subscribing in-between notifications.
+func TestChannelObservable_ObserversSubscribeSerially(t *testing.T) {
+	t.Skip("add coverage: observers which subscribe in-between notifications should receive specific value ranges")
+}
+
+type observation[V any] struct {
+	sync.Mutex
+	observable.Observer[V]
+	Notifications *[]V
+}
+
+func newObservation[V any](
+	ctx context.Context,
+	observable observable.Observable[V],
+) *observation[V] {
+	return &observation[V]{
+		Observer:      observable.Subscribe(ctx),
+		Notifications: new([]V),
+	}
+}
+
+func (o *observation[V]) notify(value V) {
+	o.Lock()
+	defer o.Unlock()
+
+	*o.Notifications = append(*o.Notifications, value)
+}
+
+func goReceiveNotifications[V any](obsvn *observation[V]) {
+	for notification := range obsvn.Ch() {
+		obsvn.notify(notification)
+	}
+}
+
+func syncSendWithDelayFactory[V any](producer chan<- V, delay time.Duration) func(value V) {
+	return func(value V) {
+		time.Sleep(delay)
+		producer <- value
+		time.Sleep(delay)
+	}
+}
+
+func goNotifiedOrTimedOutFactory[V any](
+	obsvr observable.Observer[V],
+	next func(index int, output V) error,
+	done func(outputs []V) error,
+) func() error {
+	var (
+		outputIndex int
+		outputs     []V
+	)
+	return func() error {
+		for {
+			select {
+			case output, ok := <-obsvr.Ch():
+				if !ok {
+					break
+				}
+
+				if err := next(outputIndex, output); err != nil {
+					return err
+				}
+
+				outputs = append(outputs, output)
+				outputIndex++
+				continue
+			case <-time.After(notifyTimeout):
+				return fmt.Errorf("timed out waiting for observer to be notified")
+			}
+
+			if err := done(outputs); err != nil {
+				return err
+			}
+			return nil
+		}
+
+	}
+}
diff --git a/pkg/observable/channel/observer_test.go b/pkg/observable/channel/observer_test.go
index b70e5bd9d..ffc105c17 100644
--- a/pkg/observable/channel/observer_test.go
+++ b/pkg/observable/channel/observer_test.go
@@ -12,7 +12,7 @@ func TestObserver_Unsubscribe(t *testing.T) {
 		onUnsubscribeCalled = false
 		inputCh             = make(chan int, 1)
 	)
-	sub := &channelObserver[int]{
+	obsvr := &channelObserver[int]{
 		observerMu: &sync.RWMutex{},
 		// using a buffered  channel to keep the test synchronous
 		observerCh: inputCh,
@@ -22,13 +22,13 @@ func TestObserver_Unsubscribe(t *testing.T) {
 	}
 
 	// should initially be open
-	require.Equal(t, false, sub.closed)
+	require.Equal(t, false, obsvr.closed)
 
 	inputCh <- 1
-	require.Equal(t, false, sub.closed)
+	require.Equal(t, false, obsvr.closed)
 
-	sub.Unsubscribe()
+	obsvr.Unsubscribe()
 	// should be closed after `#Unsubscribe()`
-	require.Equal(t, true, sub.closed)
+	require.Equal(t, true, obsvr.closed)
 	require.True(t, onUnsubscribeCalled)
 }

From c035b5c1f884cadc5cdcd9dec25c42a0a2c5c9bf Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Sat, 14 Oct 2023 01:21:42 +0200
Subject: [PATCH 14/41] test: cleanup & comment observable tests

---
 pkg/observable/channel/observable_test.go  | 57 +++-------------------
 pkg/observable/channel/observation_test.go | 37 ++++++++++++++
 pkg/observable/channel/observer.go         |  4 +-
 3 files changed, 47 insertions(+), 51 deletions(-)
 create mode 100644 pkg/observable/channel/observation_test.go

diff --git a/pkg/observable/channel/observable_test.go b/pkg/observable/channel/observable_test.go
index 186540fb4..c7a605775 100644
--- a/pkg/observable/channel/observable_test.go
+++ b/pkg/observable/channel/observable_test.go
@@ -3,7 +3,6 @@ package channel_test
 import (
 	"context"
 	"fmt"
-	"sync"
 	"testing"
 	"time"
 
@@ -94,6 +93,7 @@ func TestChannelObservable_NotifyObservers(t *testing.T) {
 			)
 			require.NotNil(t, obsvbl)
 			require.NotNil(t, producer)
+			produceWithDelay := syncSendWithDelayFactory(producer, productionDelay)
 
 			// construct 3 distinct observers, each with its own channel
 			observers := make([]observable.Observer[*int], 1)
@@ -140,9 +140,10 @@ func TestChannelObservable_NotifyObservers(t *testing.T) {
 				inputPtr := new(int)
 				*inputPtr = input
 				t.Logf("sending input ptr: %d %p", input, inputPtr)
-				time.Sleep(productionDelay)
-				producer <- inputPtr
-				time.Sleep(productionDelay)
+
+				// simulating IO delay in sequential message production
+				produceWithDelay(inputPtr)
+
 				t.Logf("send input %d", i)
 			}
 			cancel()
@@ -257,12 +258,7 @@ func TestChannelObservable_ConcurrentSubUnSub(t *testing.T) {
 	t.Skip("add coverage: subscribing and unsubscribing concurrently should not race")
 }
 
-// TECHDEBT/INCOMPLETE: add coverage for multiple observers, unsubscribe from one
-// and ensure the rest are still notified.
-// TODO_THIS_COMMIT: consider renaming, also has to do with sequential notificaions
-func TestChannelObservable_ObserversUnsubscribeIndependently(t *testing.T) {
-	//t.Skip("add coverage: unsubscribing one observer should not impact the rest")
-
+func TestChannelObservable_SequentialProductionAndUnsubscription(t *testing.T) {
 	observations := new([]*observation[int])
 	expectedNotifications := [][]int{
 		{123, 456, 789},
@@ -271,14 +267,10 @@ func TestChannelObservable_ObserversUnsubscribeIndependently(t *testing.T) {
 		{987, 654, 321},
 	}
 
-	//type observation struct {
-	//	observerIndex int
-	//	notifications []int
-	//}
-
 	obsvbl, producer := channel.NewObservable[int]()
 	require.NotNil(t, obsvbl)
 	require.NotNil(t, producer)
+	// simulate IO delay in sequential message production
 	produceWithDelay := syncSendWithDelayFactory(producer, productionDelay)
 
 	ctx, cancel := context.WithCancel(context.Background())
@@ -326,6 +318,7 @@ func TestChannelObservable_ObserversUnsubscribeIndependently(t *testing.T) {
 
 			obsrvn.Lock()
 			defer obsrvn.Unlock()
+
 			require.Equalf(
 				t, len(expectedNotifications[obsnIdx]),
 				len(*obsrvn.Notifications),
@@ -349,40 +342,6 @@ func TestChannelObservable_ObserversCloseOnProducerClose(t *testing.T) {
 	t.Skip("add coverage: all observers should close when producer closes")
 }
 
-// TECHDEBT/INCOMPLETE: add coverage for observers (un)subscribing in-between notifications.
-func TestChannelObservable_ObserversSubscribeSerially(t *testing.T) {
-	t.Skip("add coverage: observers which subscribe in-between notifications should receive specific value ranges")
-}
-
-type observation[V any] struct {
-	sync.Mutex
-	observable.Observer[V]
-	Notifications *[]V
-}
-
-func newObservation[V any](
-	ctx context.Context,
-	observable observable.Observable[V],
-) *observation[V] {
-	return &observation[V]{
-		Observer:      observable.Subscribe(ctx),
-		Notifications: new([]V),
-	}
-}
-
-func (o *observation[V]) notify(value V) {
-	o.Lock()
-	defer o.Unlock()
-
-	*o.Notifications = append(*o.Notifications, value)
-}
-
-func goReceiveNotifications[V any](obsvn *observation[V]) {
-	for notification := range obsvn.Ch() {
-		obsvn.notify(notification)
-	}
-}
-
 func syncSendWithDelayFactory[V any](producer chan<- V, delay time.Duration) func(value V) {
 	return func(value V) {
 		time.Sleep(delay)
diff --git a/pkg/observable/channel/observation_test.go b/pkg/observable/channel/observation_test.go
new file mode 100644
index 000000000..a6bfa41fc
--- /dev/null
+++ b/pkg/observable/channel/observation_test.go
@@ -0,0 +1,37 @@
+package channel_test
+
+import (
+	"context"
+	"sync"
+
+	"pocket/pkg/observable"
+)
+
+type observation[V any] struct {
+	sync.Mutex
+	observable.Observer[V]
+	Notifications *[]V
+}
+
+func newObservation[V any](
+	ctx context.Context,
+	observable observable.Observable[V],
+) *observation[V] {
+	return &observation[V]{
+		Observer:      observable.Subscribe(ctx),
+		Notifications: new([]V),
+	}
+}
+
+func (o *observation[V]) notify(value V) {
+	o.Lock()
+	defer o.Unlock()
+
+	*o.Notifications = append(*o.Notifications, value)
+}
+
+func goReceiveNotifications[V any](obsvn *observation[V]) {
+	for notification := range obsvn.Ch() {
+		obsvn.notify(notification)
+	}
+}
diff --git a/pkg/observable/channel/observer.go b/pkg/observable/channel/observer.go
index 86f202fa2..46679b49b 100644
--- a/pkg/observable/channel/observer.go
+++ b/pkg/observable/channel/observer.go
@@ -81,8 +81,8 @@ func (obsvr *channelObserver[V]) Ch() <-chan V {
 	return obsvr.observerCh
 }
 
-// TODO_CLEANUP_COMMENT: used by observable to send to subscriber  channel
-// because channelObserver#Ch returns a receive-only channel
+// notify is used called by observable to send on the observer channel. Can't
+// use channelObserver#Ch because it's receive-only.
 func (obsvr *channelObserver[V]) notify(value V) {
 	obsvr.observerMu.Lock()
 	defer obsvr.observerMu.Unlock()

From 3a99221d19c9a7a0f9b0f034999748d4893f590c Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Tue, 17 Oct 2023 21:17:22 +0200
Subject: [PATCH 15/41] fixup: observable

(cherry picked from commit 33f3196535b7dae154e01f93aab36f70cda8fc4f)
---
 pkg/observable/channel/observable.go | 65 +++++++++++++------
 pkg/observable/channel/observer.go   | 97 ++++++++++++++++++----------
 2 files changed, 107 insertions(+), 55 deletions(-)

diff --git a/pkg/observable/channel/observable.go b/pkg/observable/channel/observable.go
index 85122593d..f229b9eb0 100644
--- a/pkg/observable/channel/observable.go
+++ b/pkg/observable/channel/observable.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"sync"
+	"time"
 
 	"pocket/pkg/observable"
 )
@@ -22,7 +23,7 @@ type channelObservable[V any] struct {
 	// observersMu protects observers from concurrent access/updates
 	observersMu *sync.RWMutex
 	// observers is a list of channelObservers that will be notified when producer
-	// receives a value. It is a pointer because ...
+	// receives a value.
 	observers []*channelObserver[V]
 }
 
@@ -46,7 +47,7 @@ func NewObservable[V any](opts ...option[V]) (observable.Observable[V], chan<- V
 	}
 
 	// start listening to the producer and emit values to observers
-	go obs.goListen(obs.producer)
+	go obs.goProduce(obs.producer)
 
 	return obs, obs.producer
 }
@@ -84,31 +85,49 @@ func (obsvbl *channelObservable[V]) Close() {
 // CONSIDERATION: decide whether this should close the producer channel; perhaps
 // only if it was provided.
 func (obsvbl *channelObservable[V]) close() {
+	// must lock in order to copy the observers list
 	obsvbl.observersMu.Lock()
-	defer obsvbl.observersMu.Unlock()
-
-	observers := obsvbl.observers
+	// copy observers to avoid holding the lock while unsubscribing them
+	var activeObservers = make([]*channelObserver[V], len(obsvbl.observers))
+	for idx, toClose := range obsvbl.observers {
+		activeObservers[idx] = toClose
+	}
+	// unlock before unsubscribing to avoid deadlock
+	obsvbl.observersMu.Unlock()
 
-	for _, obsvr := range observers {
-		fmt.Printf("channelObservable#goListen: unsubscribing %p\n", obsvr)
-		obsvr.Unsubscribe()
+	for _, observer := range activeObservers {
+		observer.Unsubscribe()
 	}
 
 	// clear observers
+	obsvbl.observersMu.Lock()
 	obsvbl.observers = []*channelObserver[V]{}
+	obsvbl.observersMu.Unlock()
 }
 
-// goListen to the producer and notify observers when values are received. This
+// goProduce to the producer and notify observers when values are received. This
 // function is blocking and should be run in a goroutine.
-func (obsvbl *channelObservable[V]) goListen(producer <-chan V) {
+func (obsvbl *channelObservable[V]) goProduce(producer <-chan V) {
+	var observers []*channelObserver[V]
 	for notification := range producer {
+		//fmt.Printf("producer received notification: %s\n", notification)
+		// TODO_THIS_COMMIT: (dis)prove the need for this in a test
 		// copy observers to avoid holding the lock while notifying
-		obsvbl.observersMu.RLock()
-		observers := make([]*channelObserver[V], len(obsvbl.observers))
-		for i, obsvr := range obsvbl.observers {
-			observers[i] = obsvr
+		for {
+			//fmt.Println("[obsersversMu] goProduce Rlocking...")
+			if !obsvbl.observersMu.TryRLock() {
+				time.Sleep(100 * time.Millisecond)
+				continue
+			}
+			observers = make([]*channelObserver[V], len(obsvbl.observers))
+			//obsvbl.observersMu.RLock()
+			//observers := make([]*channelObserver[V], len(obsvbl.observers))
+			for i, obsvr := range obsvbl.observers {
+				observers[i] = obsvr
+			}
+			obsvbl.observersMu.RUnlock()
+			break
 		}
-		obsvbl.observersMu.RUnlock()
 
 		// notify observers
 		for _, obsvr := range observers {
@@ -127,6 +146,7 @@ func (obsvbl *channelObservable[V]) goListen(producer <-chan V) {
 // It is blocking and intended to be called in a goroutine.
 func goUnsubscribeOnDone[V any](ctx context.Context, subscription observable.Observer[V]) {
 	<-ctx.Done()
+	fmt.Println("goUnsubscribeOnDone: context done")
 	subscription.Unsubscribe()
 }
 
@@ -134,12 +154,17 @@ func goUnsubscribeOnDone[V any](ctx context.Context, subscription observable.Obs
 // observable's list of observers.
 func (obsvbl *channelObservable[V]) onUnsubscribeFactory() UnsubscribeFunc[V] {
 	return func(toRemove *channelObserver[V]) {
+		fmt.Println("[observersMu] onUnsubscribeFactory() locking... ")
 		obsvbl.observersMu.Lock()
-		defer obsvbl.observersMu.Unlock()
-		observers := obsvbl.observers
-
-		for i, subscription := range observers {
-			if subscription == toRemove {
+		fmt.Println("[observersMu] ...onUnsubscribeFactory() locked ")
+		defer func() {
+			fmt.Println("[observersMu] onUnsubscribeFactory() unlocking... ")
+			obsvbl.observersMu.Unlock()
+			fmt.Println("[observersMu] ...onUnsubscribeFactory() unlocked ")
+		}()
+
+		for i, observer := range obsvbl.observers {
+			if observer == toRemove {
 				obsvbl.observers = append((obsvbl.observers)[:i], (obsvbl.observers)[i+1:]...)
 				break
 			}
diff --git a/pkg/observable/channel/observer.go b/pkg/observable/channel/observer.go
index 46679b49b..bb4a85def 100644
--- a/pkg/observable/channel/observer.go
+++ b/pkg/observable/channel/observer.go
@@ -2,17 +2,26 @@ package channel
 
 import (
 	"context"
-	"fmt"
 	"sync"
+	"time"
 
 	"pocket/pkg/observable"
 )
 
-// DISCUSS: what should this be? should it be configurable? It seems to be most
-// relevant in the context of the behavior of the observable when it has multiple
-// observers which consume at different rates.
-// observerBufferSize is the buffer size of a channelObserver's channel.
-const observerBufferSize = 1
+const (
+	// DISCUSS: what should this be? should it be configurable? It seems to be most
+	// relevant in the context of the behavior of the observable when it has multiple
+	// observers which consume at different rates.
+	// observerBufferSize is the buffer size of a channelObserver's channel.
+	observerBufferSize = 1
+	// sendRetryInterval is the duration between attempts to send on the observer's
+	// channel in the event that it's full. It facilitates a branch in a for loop
+	// which unlocks the observer's mutex and tries again.
+	// NOTE: setting this too low can cause the send retry loop to "slip", giving
+	// up on a send attempt before the channel is ready to receive for multiple
+	// iterations of the loop.
+	sendRetryInterval = 100 * time.Millisecond
+)
 
 var _ observable.Observer[any] = &channelObserver[any]{}
 
@@ -41,9 +50,6 @@ func NewObserver[V any](
 	onUnsubscribeFactory UnsubscribeFactory[V],
 ) *channelObserver[V] {
 	// Create a channel for the subscriber and append it to the observers list
-	ch := make(chan V, 1)
-	fmt.Printf("channelObservable#Subscribe: opening %p\n", ch)
-
 	return &channelObserver[V]{
 		ctx:           ctx,
 		observerMu:    new(sync.RWMutex),
@@ -55,6 +61,17 @@ func NewObserver[V any](
 // Unsubscribe closes the subscription channel and removes the subscription from
 // the observable.
 func (obsvr *channelObserver[V]) Unsubscribe() {
+	obsvr.unsubscribe()
+}
+
+// Ch returns a receive-only subscription channel.
+func (obsvr *channelObserver[V]) Ch() <-chan V {
+	return obsvr.observerCh
+}
+
+// unsubscribe closes the subscription channel, marks the observer as closed, and
+// removes the subscription from its observable's observers list via onUnsubscribe.
+func (obsvr *channelObserver[V]) unsubscribe() {
 	obsvr.observerMu.Lock()
 	defer func() {
 		obsvr.observerMu.Unlock()
@@ -64,38 +81,48 @@ func (obsvr *channelObserver[V]) Unsubscribe() {
 		return
 	}
 
-	fmt.Printf("channelObserver#Unsubscribe: closing %p\n", obsvr.observerCh)
 	close(obsvr.observerCh)
 	obsvr.closed = true
-
 	obsvr.onUnsubscribe(obsvr)
 }
 
-// Ch returns a receive-only subscription channel.
-func (obsvr *channelObserver[V]) Ch() <-chan V {
-	obsvr.observerMu.Lock()
-	defer func() {
-		obsvr.observerMu.Unlock()
-	}()
-
-	return obsvr.observerCh
-}
-
 // notify is used called by observable to send on the observer channel. Can't
-// use channelObserver#Ch because it's receive-only.
+// use channelObserver#Ch because it's receive-only. It will block if the channel
+// is full but will release the read-lock for half of the sendRetryInterval. The
+// other half holds is spent holding the read-lock and waiting for the (full)
+// channel to be ready to receive.
 func (obsvr *channelObserver[V]) notify(value V) {
-	obsvr.observerMu.Lock()
-	defer obsvr.observerMu.Unlock()
-
-	if obsvr.closed {
-		return
-	}
-
-	select {
-	case obsvr.observerCh <- value:
-	case <-obsvr.ctx.Done():
-		// TECHDEBT: add a  default path which buffers values so that the sender
-		// doesn't block and other consumers can still receive.
-		// TECHDEBT: add some logic to drain the buffer at some appropriate time
+	defer obsvr.observerMu.RUnlock()
+
+	// if observerCh is full, release the lock and try again every sendRetryInterval.
+	sendRetryTicker := time.NewTicker(sendRetryInterval / 2)
+	for {
+		// observerMu must remain read-locked until the value is sent on observerCh
+		if !obsvr.observerMu.TryRLock() {
+			time.Sleep(sendRetryInterval)
+			continue
+		}
+		if obsvr.closed {
+			return
+		}
+
+		select {
+		case <-obsvr.ctx.Done():
+			// if the context is done just release the read-lock (deferred)
+			return
+		case obsvr.observerCh <- value:
+			return
+		case <-sendRetryTicker.C:
+			// CONSIDERATION: repurpose this retry loop into a default path which
+			// buffers values so that the producer doesn't block and other observers
+			// can still be notified.
+			// TECHDEBT: add some logic to drain the buffer at some appropriate time
+
+			// if the context isn't done and channel is full (i.e. blocking),
+			// release the read-lock to give writer-lockers a turn. This case
+			// continues the loop, re-read-locking and trying again.
+			obsvr.observerMu.RUnlock()
+		}
+		time.Sleep(sendRetryInterval / 2)
 	}
 }

From aaea7b904d49431140e6b48874f86440186546f8 Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Tue, 17 Oct 2023 21:17:26 +0200
Subject: [PATCH 16/41] fixup: observable test

(cherry picked from commit 9c206da115dc35843d588313c2215a0e649c6df6)
---
 pkg/observable/channel/observable_test.go |  1 +
 pkg/observable/channel/observer_test.go   | 46 +++++++++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/pkg/observable/channel/observable_test.go b/pkg/observable/channel/observable_test.go
index c7a605775..784248a69 100644
--- a/pkg/observable/channel/observable_test.go
+++ b/pkg/observable/channel/observable_test.go
@@ -240,6 +240,7 @@ func TestChannelObservable_UnsubscribeObservers(t *testing.T) {
 	}
 }
 
+// TODO_THIS_COMMIT: move & de-dup
 func drainCh[V any](ch <-chan V) (closed bool, err error) {
 	for {
 		select {
diff --git a/pkg/observable/channel/observer_test.go b/pkg/observable/channel/observer_test.go
index ffc105c17..2534209fd 100644
--- a/pkg/observable/channel/observer_test.go
+++ b/pkg/observable/channel/observer_test.go
@@ -1,8 +1,10 @@
 package channel
 
 import (
+	"context"
 	"sync"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/require"
 )
@@ -32,3 +34,47 @@ func TestObserver_Unsubscribe(t *testing.T) {
 	require.Equal(t, true, obsvr.closed)
 	require.True(t, onUnsubscribeCalled)
 }
+
+func TestObserver_ConcurrentUnsubscribe(t *testing.T) {
+	var (
+		onUnsubscribeCalled = false
+		inputCh             = make(chan int, 1)
+	)
+	obsvr := &channelObserver[int]{
+		ctx:        context.Background(),
+		observerMu: &sync.RWMutex{},
+		// using a buffered  channel to keep the test synchronous
+		observerCh: inputCh,
+		onUnsubscribe: func(toRemove *channelObserver[int]) {
+			onUnsubscribeCalled = true
+		},
+	}
+
+	// should initially be open
+	require.Equal(t, false, obsvr.closed)
+
+	done := make(chan struct{}, 1)
+	go func() {
+		for inputIdx := 0; ; inputIdx++ {
+			select {
+			case <-done:
+				return
+			default:
+			}
+
+			obsvr.notify(inputIdx)
+			//time.Sleep(50 * time.Millisecond)
+		}
+	}()
+	t.Cleanup(func() { done <- struct{}{} })
+
+	// wait a bit, then assert that the observer is still open
+	time.Sleep(50 * time.Millisecond)
+
+	require.Equal(t, false, obsvr.closed)
+
+	obsvr.Unsubscribe()
+	// should be closed after `#Unsubscribe()`
+	require.Equal(t, true, obsvr.closed)
+	require.True(t, onUnsubscribeCalled)
+}

From 678d25e30b93fc897514b4400012b07cb03d9e67 Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Wed, 18 Oct 2023 00:15:01 +0200
Subject: [PATCH 17/41] refactor: simplify & cleanup

---
 pkg/observable/channel/observable.go      | 29 +++++++++--------------
 pkg/observable/channel/observable_test.go |  3 ---
 2 files changed, 11 insertions(+), 21 deletions(-)

diff --git a/pkg/observable/channel/observable.go b/pkg/observable/channel/observable.go
index f229b9eb0..c49754796 100644
--- a/pkg/observable/channel/observable.go
+++ b/pkg/observable/channel/observable.go
@@ -66,7 +66,7 @@ func (obsvbl *channelObservable[V]) Subscribe(ctx context.Context) observable.Ob
 	obsvbl.observersMu.Lock()
 	defer obsvbl.observersMu.Unlock()
 
-	observer := NewObserver[V](ctx, obsvbl.onUnsubscribeFactory)
+	observer := NewObserver[V](ctx, obsvbl.onUnsubscribe)
 	obsvbl.observers = append(obsvbl.observers, observer)
 
 	// caller can rely on context cancellation or call Close() to unsubscribe
@@ -150,24 +150,17 @@ func goUnsubscribeOnDone[V any](ctx context.Context, subscription observable.Obs
 	subscription.Unsubscribe()
 }
 
-// onUnsubscribeFactory returns a function that removes a given channelObserver from the
+// onUnsubscribe returns a function that removes a given channelObserver from the
 // observable's list of observers.
-func (obsvbl *channelObservable[V]) onUnsubscribeFactory() UnsubscribeFunc[V] {
-	return func(toRemove *channelObserver[V]) {
-		fmt.Println("[observersMu] onUnsubscribeFactory() locking... ")
-		obsvbl.observersMu.Lock()
-		fmt.Println("[observersMu] ...onUnsubscribeFactory() locked ")
-		defer func() {
-			fmt.Println("[observersMu] onUnsubscribeFactory() unlocking... ")
-			obsvbl.observersMu.Unlock()
-			fmt.Println("[observersMu] ...onUnsubscribeFactory() unlocked ")
-		}()
-
-		for i, observer := range obsvbl.observers {
-			if observer == toRemove {
-				obsvbl.observers = append((obsvbl.observers)[:i], (obsvbl.observers)[i+1:]...)
-				break
-			}
+func (obsvbl *channelObservable[V]) onUnsubscribe(toRemove *channelObserver[V]) {
+	// must lock to iterato over and modify observers list
+	obsvbl.observersMu.Lock()
+	defer obsvbl.observersMu.Unlock()
+
+	for i, observer := range obsvbl.observers {
+		if observer == toRemove {
+			obsvbl.observers = append((obsvbl.observers)[:i], (obsvbl.observers)[i+1:]...)
+			break
 		}
 	}
 }
diff --git a/pkg/observable/channel/observable_test.go b/pkg/observable/channel/observable_test.go
index 784248a69..9160ef787 100644
--- a/pkg/observable/channel/observable_test.go
+++ b/pkg/observable/channel/observable_test.go
@@ -82,8 +82,6 @@ func TestChannelObservable_NotifyObservers(t *testing.T) {
 				tt.setupFn(tt)
 			}
 
-			// TECHDEBT/INCOMPLETE: test w/ & w/o context cancellation
-			//ctx := context.Background()
 			ctx, cancel := context.WithCancel(context.Background())
 			t.Cleanup(cancel)
 
@@ -107,7 +105,6 @@ func TestChannelObservable_NotifyObservers(t *testing.T) {
 			for obsvrIdx, obsvr := range observers {
 				next := func(outputIndex int, output *int) error {
 					// obsvr channel should receive notified input
-					t.Logf("output: %d | %p", *output, output)
 					if !assert.Equalf(
 						t, tt.expectedOutputs[outputIndex],
 						*output,

From 631c453414bc0a6d00064a7e7171f3253e3e8158 Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Wed, 18 Oct 2023 00:15:09 +0200
Subject: [PATCH 18/41] chore: cleanup logs & comments

---
 pkg/observable/channel/observable.go | 5 +++--
 pkg/observable/channel/observer.go   | 9 +++------
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/pkg/observable/channel/observable.go b/pkg/observable/channel/observable.go
index c49754796..5a8aa40f4 100644
--- a/pkg/observable/channel/observable.go
+++ b/pkg/observable/channel/observable.go
@@ -52,8 +52,8 @@ func NewObservable[V any](opts ...option[V]) (observable.Observable[V], chan<- V
 	return obs, obs.producer
 }
 
-// WithProducer returns an option function sets the given producer in an observable
-// when passed to NewObservable().
+// WithProducer returns an option function which sets the given producer of the
+// resulting observable when passed to NewObservable().
 func WithProducer[V any](producer chan V) option[V] {
 	return func(obs *channelObservable[V]) {
 		obs.producer = producer
@@ -63,6 +63,7 @@ func WithProducer[V any](producer chan V) option[V] {
 // Subscribe returns an observer which is notified when the producer channel
 // receives a value.
 func (obsvbl *channelObservable[V]) Subscribe(ctx context.Context) observable.Observer[V] {
+	// must lock observersMu so that we can safely append to the observers list
 	obsvbl.observersMu.Lock()
 	defer obsvbl.observersMu.Unlock()
 
diff --git a/pkg/observable/channel/observer.go b/pkg/observable/channel/observer.go
index bb4a85def..2f455f1d5 100644
--- a/pkg/observable/channel/observer.go
+++ b/pkg/observable/channel/observer.go
@@ -42,19 +42,18 @@ type channelObserver[V any] struct {
 	closed bool
 }
 
-type UnsubscribeFactory[V any] func() UnsubscribeFunc[V]
 type UnsubscribeFunc[V any] func(toRemove *channelObserver[V])
 
 func NewObserver[V any](
 	ctx context.Context,
-	onUnsubscribeFactory UnsubscribeFactory[V],
+	onUnsubscribe UnsubscribeFunc[V],
 ) *channelObserver[V] {
 	// Create a channel for the subscriber and append it to the observers list
 	return &channelObserver[V]{
 		ctx:           ctx,
 		observerMu:    new(sync.RWMutex),
 		observerCh:    make(chan V, observerBufferSize),
-		onUnsubscribe: onUnsubscribeFactory(),
+		onUnsubscribe: onUnsubscribe,
 	}
 }
 
@@ -73,9 +72,7 @@ func (obsvr *channelObserver[V]) Ch() <-chan V {
 // removes the subscription from its observable's observers list via onUnsubscribe.
 func (obsvr *channelObserver[V]) unsubscribe() {
 	obsvr.observerMu.Lock()
-	defer func() {
-		obsvr.observerMu.Unlock()
-	}()
+	defer obsvr.observerMu.Unlock()
 
 	if obsvr.closed {
 		return

From 9340e82a6bd8f5232b59dd1f94206e46b9e77abc Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Wed, 18 Oct 2023 09:19:20 +0200
Subject: [PATCH 19/41] chore: improve comments

---
 pkg/observable/channel/observable.go | 2 +-
 pkg/observable/channel/observer.go   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/pkg/observable/channel/observable.go b/pkg/observable/channel/observable.go
index 5a8aa40f4..b029f1bf6 100644
--- a/pkg/observable/channel/observable.go
+++ b/pkg/observable/channel/observable.go
@@ -31,7 +31,7 @@ type channelObservable[V any] struct {
 // receives a value.
 // func NewObservable[V any](producer chan V) (observable.Observable[V], chan<- V) {
 func NewObservable[V any](opts ...option[V]) (observable.Observable[V], chan<- V) {
-	// initialize an observer that publishes messages from 1 producer to N observers
+	// initialize an observable that publishes messages from 1 producer to N observers
 	obs := &channelObservable[V]{
 		observersMu: &sync.RWMutex{},
 		observers:   []*channelObserver[V]{},
diff --git a/pkg/observable/channel/observer.go b/pkg/observable/channel/observer.go
index 2f455f1d5..12a0d3482 100644
--- a/pkg/observable/channel/observer.go
+++ b/pkg/observable/channel/observer.go
@@ -48,7 +48,7 @@ func NewObserver[V any](
 	ctx context.Context,
 	onUnsubscribe UnsubscribeFunc[V],
 ) *channelObserver[V] {
-	// Create a channel for the subscriber and append it to the observers list
+	// Create a channel for the observer and append it to the observers list
 	return &channelObserver[V]{
 		ctx:           ctx,
 		observerMu:    new(sync.RWMutex),

From b072eebe6a84bcae633d7fc1a37446a6039edd1e Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Wed, 18 Oct 2023 09:19:34 +0200
Subject: [PATCH 20/41] refactor: DrainChannel test helper

---
 internal/testchannel/drain.go             | 17 +++++++++++++++++
 pkg/observable/channel/observable_test.go | 17 +----------------
 2 files changed, 18 insertions(+), 16 deletions(-)
 create mode 100644 internal/testchannel/drain.go

diff --git a/internal/testchannel/drain.go b/internal/testchannel/drain.go
new file mode 100644
index 000000000..83b7375aa
--- /dev/null
+++ b/internal/testchannel/drain.go
@@ -0,0 +1,17 @@
+package testchannel
+
+import "fmt"
+
+func DrainChannel[V any](ch <-chan V) (closed bool, err error) {
+	for {
+		select {
+		case _, ok := <-ch:
+			if !ok {
+				return true, nil
+			}
+			continue
+		default:
+			return false, fmt.Errorf("observer channel left open")
+		}
+	}
+}
diff --git a/pkg/observable/channel/observable_test.go b/pkg/observable/channel/observable_test.go
index 9160ef787..edeb74197 100644
--- a/pkg/observable/channel/observable_test.go
+++ b/pkg/observable/channel/observable_test.go
@@ -156,7 +156,7 @@ func TestChannelObservable_NotifyObservers(t *testing.T) {
 				t.Logf("unsusbscribed %d", i)
 
 				// must drain the channel first to ensure it is closed
-				closed, err := drainCh(observer.Ch())
+				closed, err := testchannel.DrainChannel(observer.Ch())
 				require.NoError(t, err)
 				require.True(t, closed)
 			}
@@ -237,21 +237,6 @@ func TestChannelObservable_UnsubscribeObservers(t *testing.T) {
 	}
 }
 
-// TODO_THIS_COMMIT: move & de-dup
-func drainCh[V any](ch <-chan V) (closed bool, err error) {
-	for {
-		select {
-		case _, ok := <-ch:
-			if !ok {
-				return true, nil
-			}
-			continue
-		default:
-			return false, fmt.Errorf("observer channel left open")
-		}
-	}
-}
-
 func TestChannelObservable_ConcurrentSubUnSub(t *testing.T) {
 	t.Skip("add coverage: subscribing and unsubscribing concurrently should not race")
 }

From a487fe0a5843a6dd2544e2f807ca91478a679dc9 Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Wed, 18 Oct 2023 09:19:42 +0200
Subject: [PATCH 21/41] shore: cleanup & simplify

---
 pkg/observable/channel/observable.go      |  2 --
 pkg/observable/channel/observable_test.go | 41 +++++++++--------------
 2 files changed, 15 insertions(+), 28 deletions(-)

diff --git a/pkg/observable/channel/observable.go b/pkg/observable/channel/observable.go
index b029f1bf6..8807342ec 100644
--- a/pkg/observable/channel/observable.go
+++ b/pkg/observable/channel/observable.go
@@ -2,7 +2,6 @@ package channel
 
 import (
 	"context"
-	"fmt"
 	"sync"
 	"time"
 
@@ -147,7 +146,6 @@ func (obsvbl *channelObservable[V]) goProduce(producer <-chan V) {
 // It is blocking and intended to be called in a goroutine.
 func goUnsubscribeOnDone[V any](ctx context.Context, subscription observable.Observer[V]) {
 	<-ctx.Done()
-	fmt.Println("goUnsubscribeOnDone: context done")
 	subscription.Unsubscribe()
 }
 
diff --git a/pkg/observable/channel/observable_test.go b/pkg/observable/channel/observable_test.go
index edeb74197..b698a0e18 100644
--- a/pkg/observable/channel/observable_test.go
+++ b/pkg/observable/channel/observable_test.go
@@ -3,6 +3,7 @@ package channel_test
 import (
 	"context"
 	"fmt"
+	"pocket/internal/testchannel"
 	"testing"
 	"time"
 
@@ -15,8 +16,8 @@ import (
 )
 
 const (
-	productionDelay          = 10 * time.Millisecond
-	notifyTimeout            = productionDelay * 4
+	productionDelay          = 2 * time.Millisecond
+	notifyTimeout            = 50 * time.Millisecond
 	unsubscribeSleepDuration = notifyTimeout * 2
 )
 
@@ -85,13 +86,12 @@ func TestChannelObservable_NotifyObservers(t *testing.T) {
 			ctx, cancel := context.WithCancel(context.Background())
 			t.Cleanup(cancel)
 
-			t.Logf("producer: %p", tt.producer)
 			obsvbl, producer := channel.NewObservable[*int](
 				channel.WithProducer(tt.producer),
 			)
 			require.NotNil(t, obsvbl)
 			require.NotNil(t, producer)
-			produceWithDelay := syncSendWithDelayFactory(producer, productionDelay)
+			produce := produceWithDelay(producer, productionDelay)
 
 			// construct 3 distinct observers, each with its own channel
 			observers := make([]observable.Observer[*int], 1)
@@ -128,32 +128,26 @@ func TestChannelObservable_NotifyObservers(t *testing.T) {
 
 				// concurrently await notification or timeout to avoid blocking on
 				// empty and/or non-buffered producers.
-				group.Go(goNotifiedOrTimedOutFactory(obsvr, next, done))
+				group.Go(goNotifiedOrTimedOutFactory(obsvr, next, done, notifyTimeout))
 			}
 
 			// notify with test input
-			t.Logf("sending to producer %p", producer)
-			for i, input := range tt.inputs {
+			for _, input := range tt.inputs {
 				inputPtr := new(int)
 				*inputPtr = input
-				t.Logf("sending input ptr: %d %p", input, inputPtr)
 
 				// simulating IO delay in sequential message production
-				produceWithDelay(inputPtr)
-
-				t.Logf("send input %d", i)
+				produce(inputPtr)
 			}
 			cancel()
 
 			// wait for obsvbl to be notified or timeout
 			err := group.Wait()
 			require.NoError(t, err)
-			t.Log("errgroup done")
 
 			// unsubscribing should close obsvr channel(s)
-			for i, observer := range observers {
+			for _, observer := range observers {
 				observer.Unsubscribe()
-				t.Logf("unsusbscribed %d", i)
 
 				// must drain the channel first to ensure it is closed
 				closed, err := testchannel.DrainChannel(observer.Ch())
@@ -254,7 +248,7 @@ func TestChannelObservable_SequentialProductionAndUnsubscription(t *testing.T) {
 	require.NotNil(t, obsvbl)
 	require.NotNil(t, producer)
 	// simulate IO delay in sequential message production
-	produceWithDelay := syncSendWithDelayFactory(producer, productionDelay)
+	produceWithDelay := produceWithDelay(producer, productionDelay)
 
 	ctx, cancel := context.WithCancel(context.Background())
 	t.Cleanup(cancel)
@@ -325,11 +319,11 @@ func TestChannelObservable_ObserversCloseOnProducerClose(t *testing.T) {
 	t.Skip("add coverage: all observers should close when producer closes")
 }
 
-func syncSendWithDelayFactory[V any](producer chan<- V, delay time.Duration) func(value V) {
+func produceWithDelay[V any](producer chan<- V, delay time.Duration) func(value V) {
 	return func(value V) {
-		time.Sleep(delay)
+		time.Sleep(delay / 2)
 		producer <- value
-		time.Sleep(delay)
+		time.Sleep(delay / 2)
 	}
 }
 
@@ -337,6 +331,7 @@ func goNotifiedOrTimedOutFactory[V any](
 	obsvr observable.Observer[V],
 	next func(index int, output V) error,
 	done func(outputs []V) error,
+	timeoutDuration time.Duration,
 ) func() error {
 	var (
 		outputIndex int
@@ -347,7 +342,7 @@ func goNotifiedOrTimedOutFactory[V any](
 			select {
 			case output, ok := <-obsvr.Ch():
 				if !ok {
-					break
+					return done(outputs)
 				}
 
 				if err := next(outputIndex, output); err != nil {
@@ -357,15 +352,9 @@ func goNotifiedOrTimedOutFactory[V any](
 				outputs = append(outputs, output)
 				outputIndex++
 				continue
-			case <-time.After(notifyTimeout):
+			case <-time.After(timeoutDuration):
 				return fmt.Errorf("timed out waiting for observer to be notified")
 			}
-
-			if err := done(outputs); err != nil {
-				return err
-			}
-			return nil
 		}
-
 	}
 }

From 8df9cbc7cb736af28413c261963f8c329da59598 Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Wed, 18 Oct 2023 09:19:46 +0200
Subject: [PATCH 22/41] test: comment out flaky test cases

---
 pkg/observable/channel/observable_test.go | 55 +++++++++++++----------
 1 file changed, 31 insertions(+), 24 deletions(-)

diff --git a/pkg/observable/channel/observable_test.go b/pkg/observable/channel/observable_test.go
index b698a0e18..4fc531b56 100644
--- a/pkg/observable/channel/observable_test.go
+++ b/pkg/observable/channel/observable_test.go
@@ -31,8 +31,9 @@ func TestChannelObservable_NotifyObservers(t *testing.T) {
 	}
 
 	inputs := []int{123, 456, 789}
-	fullBlockingProducer := make(chan *int)
-	fullBufferedProducer := make(chan *int, 1)
+	// NB: see INCOMPLETE comment below
+	// fullBlockingProducer := make(chan *int)
+	// fullBufferedProducer := make(chan *int, 1)
 
 	tests := []test{
 		{
@@ -47,34 +48,40 @@ func TestChannelObservable_NotifyObservers(t *testing.T) {
 			inputs:          inputs,
 			expectedOutputs: inputs,
 		},
-		{
-			name:            "full non-buffered producer",
-			producer:        fullBlockingProducer,
-			inputs:          inputs[1:],
-			expectedOutputs: inputs,
-			setupFn: func(t test) {
-				go func() {
-					// blocking send
-					t.producer <- &inputs[0]
-				}()
-			},
-		},
 		{
 			name:            "empty buffered len 1 producer",
 			producer:        make(chan *int, 1),
 			inputs:          inputs,
 			expectedOutputs: inputs,
 		},
-		{
-			name:            "full buffered len 1 producer",
-			producer:        fullBufferedProducer,
-			inputs:          inputs[1:],
-			expectedOutputs: inputs,
-			setupFn: func(t test) {
-				// non-blocking send
-				t.producer <- &inputs[0]
-			},
-		},
+		// INCOMPLETE: producer channels which are full are proving harder to test
+		// robustly (no flakiness); perhaps it has to do with the lack of some
+		// kind of guarantee about the receiver order on the consumer side.
+		//
+		// The following scenarios should generally pass but are flaky:
+		//
+		// {
+		// 	name:            "full non-buffered producer",
+		// 	producer:        fullBlockingProducer,
+		// 	inputs:          inputs[1:],
+		// 	expectedOutputs: inputs,
+		// 	setupFn: func(t test) {
+		// 		go func() {
+		// 			// blocking send
+		// 			t.producer <- &inputs[0]
+		// 		}()
+		// 	},
+		// },
+		// {
+		// 	name:            "full buffered len 1 producer",
+		// 	producer:        fullBufferedProducer,
+		// 	inputs:          inputs[1:],
+		// 	expectedOutputs: inputs,
+		// 	setupFn: func(t test) {
+		// 		// non-blocking send
+		// 		t.producer <- &inputs[0]
+		// 	},
+		// },
 	}
 
 	for _, tt := range tests {

From 466c508f362999a94e1a1636dcfd54ffeb8e2d1a Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Wed, 18 Oct 2023 09:32:54 +0200
Subject: [PATCH 23/41] fixup: drain channel helper

---
 internal/testchannel/drain.go             | 17 +++++++++++++----
 pkg/observable/channel/observable_test.go |  3 +--
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/internal/testchannel/drain.go b/internal/testchannel/drain.go
index 83b7375aa..ad204d774 100644
--- a/internal/testchannel/drain.go
+++ b/internal/testchannel/drain.go
@@ -1,17 +1,26 @@
 package testchannel
 
-import "fmt"
+import (
+	"errors"
+)
 
-func DrainChannel[V any](ch <-chan V) (closed bool, err error) {
+var errChanNotClosed = errors.New("channel is not closed")
+
+// DrainChannel attempts to receive from the given channel, blocking, until it is
+// empty. It returns an error if the channel is not closed by the time it's empty.
+// CONSIDERATION: this function could easily take a timeout parameter and add
+// a case which returns an error if the timeout is exceeded. This would prevent
+// the case where the channel never stops receiving from looping indefinitely.
+func DrainChannel[V any](ch <-chan V) error {
 	for {
 		select {
 		case _, ok := <-ch:
 			if !ok {
-				return true, nil
+				return nil
 			}
 			continue
 		default:
-			return false, fmt.Errorf("observer channel left open")
+			return errChanNotClosed
 		}
 	}
 }
diff --git a/pkg/observable/channel/observable_test.go b/pkg/observable/channel/observable_test.go
index 4fc531b56..3a1d5f6c2 100644
--- a/pkg/observable/channel/observable_test.go
+++ b/pkg/observable/channel/observable_test.go
@@ -157,9 +157,8 @@ func TestChannelObservable_NotifyObservers(t *testing.T) {
 				observer.Unsubscribe()
 
 				// must drain the channel first to ensure it is closed
-				closed, err := testchannel.DrainChannel(observer.Ch())
+				err := testchannel.DrainChannel(observer.Ch())
 				require.NoError(t, err)
-				require.True(t, closed)
 			}
 		})
 	}

From 7a399f89467996fd89860e346741fcdd29ab7ffc Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Wed, 18 Oct 2023 09:46:08 +0200
Subject: [PATCH 24/41] chore: improve var name

---
 pkg/observable/channel/observable_test.go | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/pkg/observable/channel/observable_test.go b/pkg/observable/channel/observable_test.go
index 3a1d5f6c2..4d6c23ab2 100644
--- a/pkg/observable/channel/observable_test.go
+++ b/pkg/observable/channel/observable_test.go
@@ -16,9 +16,9 @@ import (
 )
 
 const (
-	productionDelay          = 2 * time.Millisecond
-	notifyTimeout            = 50 * time.Millisecond
-	unsubscribeSleepDuration = notifyTimeout * 2
+	productionDelay        = 2 * time.Millisecond
+	notifyTimeout          = 50 * time.Millisecond
+	cancelUnsubscribeDelay = notifyTimeout * 2
 )
 
 func TestChannelObservable_NotifyObservers(t *testing.T) {
@@ -205,7 +205,7 @@ func TestChannelObservable_UnsubscribeObservers(t *testing.T) {
 			lifecycleFn: func() observable.Observer[int] {
 				observer := obsvbl.Subscribe(ctx)
 				cancel()
-				time.Sleep(unsubscribeSleepDuration)
+				time.Sleep(cancelUnsubscribeDelay)
 				observer.Unsubscribe()
 				return observer
 			},
@@ -215,7 +215,7 @@ func TestChannelObservable_UnsubscribeObservers(t *testing.T) {
 			lifecycleFn: func() observable.Observer[int] {
 				observer := obsvbl.Subscribe(ctx)
 				observer.Unsubscribe()
-				time.Sleep(unsubscribeSleepDuration)
+				time.Sleep(cancelUnsubscribeDelay)
 				cancel()
 				return observer
 			},

From d27e114aeef71e4ace41101c056cad79581e3c7f Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Wed, 18 Oct 2023 09:47:10 +0200
Subject: [PATCH 25/41] fixup: drain channel helper

---
 pkg/observable/channel/observable_test.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pkg/observable/channel/observable_test.go b/pkg/observable/channel/observable_test.go
index 4d6c23ab2..dda1186dd 100644
--- a/pkg/observable/channel/observable_test.go
+++ b/pkg/observable/channel/observable_test.go
@@ -3,7 +3,6 @@ package channel_test
 import (
 	"context"
 	"fmt"
-	"pocket/internal/testchannel"
 	"testing"
 	"time"
 
@@ -11,6 +10,7 @@ import (
 	"github.com/stretchr/testify/require"
 	"golang.org/x/sync/errgroup"
 
+	"pocket/internal/testchannel"
 	"pocket/pkg/observable"
 	"pocket/pkg/observable/channel"
 )

From cf8507d242b7e996ef15adadac14f0436e42d728 Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Wed, 18 Oct 2023 09:49:13 +0200
Subject: [PATCH 26/41] test: shorten timeout

---
 pkg/observable/channel/observable_test.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pkg/observable/channel/observable_test.go b/pkg/observable/channel/observable_test.go
index dda1186dd..7c9667d97 100644
--- a/pkg/observable/channel/observable_test.go
+++ b/pkg/observable/channel/observable_test.go
@@ -17,7 +17,7 @@ import (
 
 const (
 	productionDelay        = 2 * time.Millisecond
-	notifyTimeout          = 50 * time.Millisecond
+	notifyTimeout          = productionDelay * 5
 	cancelUnsubscribeDelay = notifyTimeout * 2
 )
 

From c28859575cfd9a0cc9d57c663ed3bd3a26ab81cb Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Wed, 18 Oct 2023 12:36:31 +0200
Subject: [PATCH 27/41] chore: cleanup

---
 pkg/observable/channel/observable.go | 8 --------
 pkg/observable/channel/observer.go   | 2 +-
 2 files changed, 1 insertion(+), 9 deletions(-)

diff --git a/pkg/observable/channel/observable.go b/pkg/observable/channel/observable.go
index 8807342ec..f6abd7c2c 100644
--- a/pkg/observable/channel/observable.go
+++ b/pkg/observable/channel/observable.go
@@ -3,7 +3,6 @@ package channel
 import (
 	"context"
 	"sync"
-	"time"
 
 	"pocket/pkg/observable"
 )
@@ -110,18 +109,11 @@ func (obsvbl *channelObservable[V]) close() {
 func (obsvbl *channelObservable[V]) goProduce(producer <-chan V) {
 	var observers []*channelObserver[V]
 	for notification := range producer {
-		//fmt.Printf("producer received notification: %s\n", notification)
-		// TODO_THIS_COMMIT: (dis)prove the need for this in a test
-		// copy observers to avoid holding the lock while notifying
 		for {
-			//fmt.Println("[obsersversMu] goProduce Rlocking...")
 			if !obsvbl.observersMu.TryRLock() {
-				time.Sleep(100 * time.Millisecond)
 				continue
 			}
 			observers = make([]*channelObserver[V], len(obsvbl.observers))
-			//obsvbl.observersMu.RLock()
-			//observers := make([]*channelObserver[V], len(obsvbl.observers))
 			for i, obsvr := range obsvbl.observers {
 				observers[i] = obsvr
 			}
diff --git a/pkg/observable/channel/observer.go b/pkg/observable/channel/observer.go
index 12a0d3482..98888813c 100644
--- a/pkg/observable/channel/observer.go
+++ b/pkg/observable/channel/observer.go
@@ -96,7 +96,7 @@ func (obsvr *channelObserver[V]) notify(value V) {
 	for {
 		// observerMu must remain read-locked until the value is sent on observerCh
 		if !obsvr.observerMu.TryRLock() {
-			time.Sleep(sendRetryInterval)
+			time.Sleep(sendRetryInterval / 2)
 			continue
 		}
 		if obsvr.closed {

From a22f813ce6c432b37932e3ad75e26ce0956bd92c Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Thu, 19 Oct 2023 17:50:47 +0200
Subject: [PATCH 28/41] chore: cleanup, simplification, review improvements

(cherry picked from commit 92a547da29ec526d415f6967ccfa5988c3f5ca1d)
---
 internal/testchannel/drain.go             |   9 +-
 pkg/observable/channel/observable.go      | 115 ++++++++++++----------
 pkg/observable/channel/observable_test.go |  54 +++++-----
 pkg/observable/channel/observer.go        |  31 +++---
 pkg/observable/channel/observer_test.go   |  16 +--
 5 files changed, 120 insertions(+), 105 deletions(-)

diff --git a/internal/testchannel/drain.go b/internal/testchannel/drain.go
index ad204d774..32288c4e1 100644
--- a/internal/testchannel/drain.go
+++ b/internal/testchannel/drain.go
@@ -2,6 +2,7 @@ package testchannel
 
 import (
 	"errors"
+	"time"
 )
 
 var errChanNotClosed = errors.New("channel is not closed")
@@ -15,11 +16,11 @@ func DrainChannel[V any](ch <-chan V) error {
 	for {
 		select {
 		case _, ok := <-ch:
-			if !ok {
-				return nil
+			if ok {
+				continue
 			}
-			continue
-		default:
+			return nil
+		case <-time.After(time.Millisecond):
 			return errChanNotClosed
 		}
 	}
diff --git a/pkg/observable/channel/observable.go b/pkg/observable/channel/observable.go
index f6abd7c2c..7c19d6d4d 100644
--- a/pkg/observable/channel/observable.go
+++ b/pkg/observable/channel/observable.go
@@ -2,9 +2,8 @@ package channel
 
 import (
 	"context"
-	"sync"
-
 	"pocket/pkg/observable"
+	"sync"
 )
 
 var _ observable.Observable[any] = &channelObservable[any]{}
@@ -13,23 +12,23 @@ var _ observable.Observable[any] = &channelObservable[any]{}
 type option[V any] func(obs *channelObservable[V])
 
 // channelObservable implements the observable.Observable interface and can be notified
-// via its corresponding producer channel.
+// via its corresponding publishCh channel.
 type channelObservable[V any] struct {
-	// producer is an observable-wide channel that is used to receive values
+	// publishCh is an observable-wide channel that is used to receive values
 	// which are subsequently re-sent to observers.
-	producer chan V
+	publishCh chan V
 	// observersMu protects observers from concurrent access/updates
 	observersMu *sync.RWMutex
-	// observers is a list of channelObservers that will be notified when producer
+	// observers is a list of channelObservers that will be notified when publishCh
 	// receives a value.
 	observers []*channelObserver[V]
 }
 
-// NewObservable creates a new observable is notified when the producer channel
+// NewObservable creates a new observable is notified when the publishCh channel
 // receives a value.
-// func NewObservable[V any](producer chan V) (observable.Observable[V], chan<- V) {
+// func NewObservable[V any](publishCh chan V) (observable.Observable[V], chan<- V) {
 func NewObservable[V any](opts ...option[V]) (observable.Observable[V], chan<- V) {
-	// initialize an observable that publishes messages from 1 producer to N observers
+	// initialize an observable that publishes messages from 1 publishCh to N observers
 	obs := &channelObservable[V]{
 		observersMu: &sync.RWMutex{},
 		observers:   []*channelObserver[V]{},
@@ -39,26 +38,26 @@ func NewObservable[V any](opts ...option[V]) (observable.Observable[V], chan<- V
 		opt(obs)
 	}
 
-	// if the caller does not provide a producer, create a new one and return it
-	if obs.producer == nil {
-		obs.producer = make(chan V)
+	// if the caller does not provide a publishCh, create a new one and return it
+	if obs.publishCh == nil {
+		obs.publishCh = make(chan V)
 	}
 
-	// start listening to the producer and emit values to observers
-	go obs.goProduce(obs.producer)
+	// start listening to the publishCh and emit values to observers
+	go obs.goProduce(obs.publishCh)
 
-	return obs, obs.producer
+	return obs, obs.publishCh
 }
 
-// WithProducer returns an option function which sets the given producer of the
+// WithProducer returns an option function which sets the given publishCh of the
 // resulting observable when passed to NewObservable().
 func WithProducer[V any](producer chan V) option[V] {
 	return func(obs *channelObservable[V]) {
-		obs.producer = producer
+		obs.publishCh = producer
 	}
 }
 
-// Subscribe returns an observer which is notified when the producer channel
+// Subscribe returns an observer which is notified when the publishCh channel
 // receives a value.
 func (obsvbl *channelObservable[V]) Subscribe(ctx context.Context) observable.Observer[V] {
 	// must lock observersMu so that we can safely append to the observers list
@@ -81,59 +80,71 @@ func (obsvbl *channelObservable[V]) Close() {
 	obsvbl.close()
 }
 
-// CONSIDERATION: decide whether this should close the producer channel; perhaps
+// TODO_CONSIDERATION: decide whether this should close the publishCh channel; perhaps
 // only if it was provided.
 func (obsvbl *channelObservable[V]) close() {
-	// must lock in order to copy the observers list
-	obsvbl.observersMu.Lock()
-	// copy observers to avoid holding the lock while unsubscribing them
-	var activeObservers = make([]*channelObserver[V], len(obsvbl.observers))
-	for idx, toClose := range obsvbl.observers {
-		activeObservers[idx] = toClose
-	}
-	// unlock before unsubscribing to avoid deadlock
-	obsvbl.observersMu.Unlock()
-
-	for _, observer := range activeObservers {
+	// Copy currentObservers to avoid holding the lock while unsubscribing them.
+	// The current observers at this time is the canonical set of observers which
+	// will be unsubscribed.
+	// New or existing Observers may (un)subscribe while the observable is closing.
+	// Any such observers won't be isClosed but will also stop receiving notifications
+	// immediately (if they receive any at all).
+	currentObservers := obsvbl.copyObservers()
+
+	for _, observer := range currentObservers {
 		observer.Unsubscribe()
 	}
 
-	// clear observers
+	// Reset observers to an empty list. This purges any observers which might have
+	// subscribed while the observable was closing.
 	obsvbl.observersMu.Lock()
 	obsvbl.observers = []*channelObserver[V]{}
 	obsvbl.observersMu.Unlock()
 }
 
-// goProduce to the producer and notify observers when values are received. This
+// goProduce to the publishCh and notify observers when values are received. This
 // function is blocking and should be run in a goroutine.
-func (obsvbl *channelObservable[V]) goProduce(producer <-chan V) {
-	var observers []*channelObserver[V]
-	for notification := range producer {
-		for {
-			if !obsvbl.observersMu.TryRLock() {
-				continue
-			}
-			observers = make([]*channelObserver[V], len(obsvbl.observers))
-			for i, obsvr := range obsvbl.observers {
-				observers[i] = obsvr
-			}
-			obsvbl.observersMu.RUnlock()
-			break
-		}
-
-		// notify observers
-		for _, obsvr := range observers {
-			// CONSIDERATION: perhaps continue trying to avoid making this
+func (obsvbl *channelObservable[V]) goProduce(publisher <-chan V) {
+	for notification := range publisher {
+		// Copy currentObservers to avoid holding the lock while notifying them.
+		// New or existing Observers may (un)subscribe while this notification
+		// is being fanned out to the "current" set  of currentObservers.
+		// The state of currentObservers at this time is the canonical set of currentObservers
+		// which receive this notification.
+		currentObservers := obsvbl.copyObservers()
+
+		// notify currentObservers
+		for _, obsvr := range currentObservers {
+			// TODO_CONSIDERATION: perhaps continue trying to avoid making this
 			// notification async as it would effectively use goroutines
-			// in memory as a buffer (with little control surface).
+			// in memory as a buffer (unbounded).
 			obsvr.notify(notification)
 		}
 	}
 
-	// Here we know that the producer has been closed, all observers should be closed as well
+	// Here we know that the publishCh has been isClosed, all currentObservers should be isClosed as well
 	obsvbl.close()
 }
 
+func (obsvbl *channelObservable[V]) copyObservers() (observers []*channelObserver[V]) {
+	defer obsvbl.observersMu.RUnlock()
+
+	// This loop blocks on acquiring a read lock on observersMu. If TryRLock
+	// fails, the loop continues until it succeeds. This is intended to give
+	// callers a guarantee that this copy operation won't contribute to a deadlock.
+	for {
+		// block until a read lock can be acquired
+		if obsvbl.observersMu.TryRLock() {
+			break
+		}
+	}
+
+	observers = make([]*channelObserver[V], len(obsvbl.observers))
+	copy(observers, obsvbl.observers)
+
+	return observers
+}
+
 // goUnsubscribeOnDone unsubscribes from the subscription when the context is.
 // It is blocking and intended to be called in a goroutine.
 func goUnsubscribeOnDone[V any](ctx context.Context, subscription observable.Observer[V]) {
diff --git a/pkg/observable/channel/observable_test.go b/pkg/observable/channel/observable_test.go
index 7c9667d97..22167b806 100644
--- a/pkg/observable/channel/observable_test.go
+++ b/pkg/observable/channel/observable_test.go
@@ -24,64 +24,64 @@ const (
 func TestChannelObservable_NotifyObservers(t *testing.T) {
 	type test struct {
 		name            string
-		producer        chan *int
+		publishCh       chan *int
 		inputs          []int
 		expectedOutputs []int
 		setupFn         func(t test)
 	}
 
 	inputs := []int{123, 456, 789}
+	fullBufferedPublisher := make(chan *int, 1)
 	// NB: see INCOMPLETE comment below
-	// fullBlockingProducer := make(chan *int)
-	// fullBufferedProducer := make(chan *int, 1)
+	//fullBlockingPublisher := make(chan *int)
 
 	tests := []test{
 		{
-			name:            "nil producer",
-			producer:        nil,
+			name:            "nil publisher",
+			publishCh:       nil,
 			inputs:          inputs,
 			expectedOutputs: inputs,
 		},
 		{
-			name:            "empty non-buffered producer",
-			producer:        make(chan *int),
+			name:            "empty non-buffered publisher",
+			publishCh:       make(chan *int),
 			inputs:          inputs,
 			expectedOutputs: inputs,
 		},
 		{
-			name:            "empty buffered len 1 producer",
-			producer:        make(chan *int, 1),
+			name:            "empty buffered len 1 publisher",
+			publishCh:       make(chan *int, 1),
 			inputs:          inputs,
 			expectedOutputs: inputs,
 		},
-		// INCOMPLETE: producer channels which are full are proving harder to test
+		{
+			name:            "full buffered len 1 publisher",
+			publishCh:       fullBufferedPublisher,
+			inputs:          inputs[1:],
+			expectedOutputs: inputs,
+			setupFn: func(t test) {
+				// non-blocking send
+				t.publishCh <- &inputs[0]
+			},
+		},
+		// INCOMPLETE: publisher channels which are full are proving harder to test
 		// robustly (no flakiness); perhaps it has to do with the lack of some
 		// kind of guarantee about the receiver order on the consumer side.
 		//
 		// The following scenarios should generally pass but are flaky:
 		//
 		// {
-		// 	name:            "full non-buffered producer",
-		// 	producer:        fullBlockingProducer,
+		// 	name:            "full non-buffered publisher",
+		// 	publishCh:       fullBlockingPublisher,
 		// 	inputs:          inputs[1:],
 		// 	expectedOutputs: inputs,
 		// 	setupFn: func(t test) {
 		// 		go func() {
 		// 			// blocking send
-		// 			t.producer <- &inputs[0]
+		// 			t.publishCh <- &inputs[0]
 		// 		}()
 		// 	},
 		// },
-		// {
-		// 	name:            "full buffered len 1 producer",
-		// 	producer:        fullBufferedProducer,
-		// 	inputs:          inputs[1:],
-		// 	expectedOutputs: inputs,
-		// 	setupFn: func(t test) {
-		// 		// non-blocking send
-		// 		t.producer <- &inputs[0]
-		// 	},
-		// },
 	}
 
 	for _, tt := range tests {
@@ -94,7 +94,7 @@ func TestChannelObservable_NotifyObservers(t *testing.T) {
 			t.Cleanup(cancel)
 
 			obsvbl, producer := channel.NewObservable[*int](
-				channel.WithProducer(tt.producer),
+				channel.WithProducer(tt.publishCh),
 			)
 			require.NotNil(t, obsvbl)
 			require.NotNil(t, producer)
@@ -156,7 +156,7 @@ func TestChannelObservable_NotifyObservers(t *testing.T) {
 			for _, observer := range observers {
 				observer.Unsubscribe()
 
-				// must drain the channel first to ensure it is closed
+				// must drain the channel first to ensure it is isClosed
 				err := testchannel.DrainChannel(observer.Ch())
 				require.NoError(t, err)
 			}
@@ -320,9 +320,9 @@ func TestChannelObservable_SequentialProductionAndUnsubscription(t *testing.T) {
 	}
 }
 
-// TECHDEBT/INCOMPLETE: add coverage for active observers closing when producer closes.
+// TECHDEBT/INCOMPLETE: add coverage for active observers closing when publishCh closes.
 func TestChannelObservable_ObserversCloseOnProducerClose(t *testing.T) {
-	t.Skip("add coverage: all observers should close when producer closes")
+	t.Skip("add coverage: all observers should close when publishCh closes")
 }
 
 func produceWithDelay[V any](producer chan<- V, delay time.Duration) func(value V) {
diff --git a/pkg/observable/channel/observer.go b/pkg/observable/channel/observer.go
index 98888813c..37c6f3071 100644
--- a/pkg/observable/channel/observer.go
+++ b/pkg/observable/channel/observer.go
@@ -9,7 +9,7 @@ import (
 )
 
 const (
-	// DISCUSS: what should this be? should it be configurable? It seems to be most
+	// TODO_DISCUSS: what should this be? should it be configurable? It seems to be most
 	// relevant in the context of the behavior of the observable when it has multiple
 	// observers which consume at different rates.
 	// observerBufferSize is the buffer size of a channelObserver's channel.
@@ -31,15 +31,15 @@ type channelObserver[V any] struct {
 	// onUnsubscribe is called in Observer#Unsubscribe, removing the respective
 	// observer from observers in a concurrency-safe manner.
 	onUnsubscribe func(toRemove *channelObserver[V])
-	// observerMu protects the observerCh and closed fields.
+	// observerMu protects the observerCh and isClosed fields.
 	observerMu *sync.RWMutex
 	// observerCh is the channel that is used to emit values to the observer.
 	// I.e. on the "N" side of the 1:N relationship between observable and
 	// observer.
 	observerCh chan V
-	// closed indicates whether the observer has been closed. It's set in
-	// unsubscribe; closed observers can't be reused.
-	closed bool
+	// isClosed indicates whether the observer has been isClosed. It's set in
+	// unsubscribe; isClosed observers can't be reused.
+	isClosed bool
 }
 
 type UnsubscribeFunc[V any] func(toRemove *channelObserver[V])
@@ -68,18 +68,18 @@ func (obsvr *channelObserver[V]) Ch() <-chan V {
 	return obsvr.observerCh
 }
 
-// unsubscribe closes the subscription channel, marks the observer as closed, and
+// unsubscribe closes the subscription channel, marks the observer as isClosed, and
 // removes the subscription from its observable's observers list via onUnsubscribe.
 func (obsvr *channelObserver[V]) unsubscribe() {
 	obsvr.observerMu.Lock()
 	defer obsvr.observerMu.Unlock()
 
-	if obsvr.closed {
+	if obsvr.isClosed {
 		return
 	}
 
 	close(obsvr.observerCh)
-	obsvr.closed = true
+	obsvr.isClosed = true
 	obsvr.onUnsubscribe(obsvr)
 }
 
@@ -91,15 +91,16 @@ func (obsvr *channelObserver[V]) unsubscribe() {
 func (obsvr *channelObserver[V]) notify(value V) {
 	defer obsvr.observerMu.RUnlock()
 
-	// if observerCh is full, release the lock and try again every sendRetryInterval.
 	sendRetryTicker := time.NewTicker(sendRetryInterval / 2)
 	for {
 		// observerMu must remain read-locked until the value is sent on observerCh
+		// in the event that it would be isClosed concurrently (i.e. this observer
+		// unsubscribes), which could cause a "send on isClosed channel" error.
 		if !obsvr.observerMu.TryRLock() {
 			time.Sleep(sendRetryInterval / 2)
 			continue
 		}
-		if obsvr.closed {
+		if obsvr.isClosed {
 			return
 		}
 
@@ -109,15 +110,17 @@ func (obsvr *channelObserver[V]) notify(value V) {
 			return
 		case obsvr.observerCh <- value:
 			return
+		// if the context isn't done and channel is full (i.e. blocking),
+		// release the read-lock to give write-lockers a turn. This case
+		// continues the loop, re-read-locking and trying again.
 		case <-sendRetryTicker.C:
 			// CONSIDERATION: repurpose this retry loop into a default path which
-			// buffers values so that the producer doesn't block and other observers
+			// buffers values so that the publishCh doesn't block and other observers
 			// can still be notified.
 			// TECHDEBT: add some logic to drain the buffer at some appropriate time
 
-			// if the context isn't done and channel is full (i.e. blocking),
-			// release the read-lock to give writer-lockers a turn. This case
-			// continues the loop, re-read-locking and trying again.
+			// this case implies that the (read) lock was acquired, so it must
+			// be unlocked before continuing the send retry loop.
 			obsvr.observerMu.RUnlock()
 		}
 		time.Sleep(sendRetryInterval / 2)
diff --git a/pkg/observable/channel/observer_test.go b/pkg/observable/channel/observer_test.go
index 2534209fd..83f05b94e 100644
--- a/pkg/observable/channel/observer_test.go
+++ b/pkg/observable/channel/observer_test.go
@@ -24,14 +24,14 @@ func TestObserver_Unsubscribe(t *testing.T) {
 	}
 
 	// should initially be open
-	require.Equal(t, false, obsvr.closed)
+	require.Equal(t, false, obsvr.isClosed)
 
 	inputCh <- 1
-	require.Equal(t, false, obsvr.closed)
+	require.Equal(t, false, obsvr.isClosed)
 
 	obsvr.Unsubscribe()
-	// should be closed after `#Unsubscribe()`
-	require.Equal(t, true, obsvr.closed)
+	// should be isClosed after `#Unsubscribe()`
+	require.Equal(t, true, obsvr.isClosed)
 	require.True(t, onUnsubscribeCalled)
 }
 
@@ -51,7 +51,7 @@ func TestObserver_ConcurrentUnsubscribe(t *testing.T) {
 	}
 
 	// should initially be open
-	require.Equal(t, false, obsvr.closed)
+	require.Equal(t, false, obsvr.isClosed)
 
 	done := make(chan struct{}, 1)
 	go func() {
@@ -71,10 +71,10 @@ func TestObserver_ConcurrentUnsubscribe(t *testing.T) {
 	// wait a bit, then assert that the observer is still open
 	time.Sleep(50 * time.Millisecond)
 
-	require.Equal(t, false, obsvr.closed)
+	require.Equal(t, false, obsvr.isClosed)
 
 	obsvr.Unsubscribe()
-	// should be closed after `#Unsubscribe()`
-	require.Equal(t, true, obsvr.closed)
+	// should be isClosed after `#Unsubscribe()`
+	require.Equal(t, true, obsvr.isClosed)
 	require.True(t, onUnsubscribeCalled)
 }

From bad5eef324bf763a9e36749142f4d92982014761 Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Thu, 19 Oct 2023 19:51:14 +0200
Subject: [PATCH 29/41] chore: improve comments

Co-authored-by: Daniel Olshansky <olshansky.daniel@gmail.com>
---
 internal/testchannel/drain.go             |  2 +-
 pkg/observable/channel/observable.go      | 12 ++++++------
 pkg/observable/channel/observable_test.go |  5 +++--
 pkg/observable/channel/observer.go        | 15 +++++++++------
 pkg/observable/channel/observer_test.go   |  4 ++--
 pkg/observable/interface.go               |  6 +++++-
 6 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/internal/testchannel/drain.go b/internal/testchannel/drain.go
index 32288c4e1..82fee2ca3 100644
--- a/internal/testchannel/drain.go
+++ b/internal/testchannel/drain.go
@@ -9,7 +9,7 @@ var errChanNotClosed = errors.New("channel is not closed")
 
 // DrainChannel attempts to receive from the given channel, blocking, until it is
 // empty. It returns an error if the channel is not closed by the time it's empty.
-// CONSIDERATION: this function could easily take a timeout parameter and add
+// TODO_CONSIDERATION: this function could easily take a timeout parameter and add
 // a case which returns an error if the timeout is exceeded. This would prevent
 // the case where the channel never stops receiving from looping indefinitely.
 func DrainChannel[V any](ch <-chan V) error {
diff --git a/pkg/observable/channel/observable.go b/pkg/observable/channel/observable.go
index 7c19d6d4d..bec168abd 100644
--- a/pkg/observable/channel/observable.go
+++ b/pkg/observable/channel/observable.go
@@ -15,12 +15,12 @@ type option[V any] func(obs *channelObservable[V])
 // via its corresponding publishCh channel.
 type channelObservable[V any] struct {
 	// publishCh is an observable-wide channel that is used to receive values
-	// which are subsequently re-sent to observers.
+	// which are subsequently fanned out to observers.
 	publishCh chan V
 	// observersMu protects observers from concurrent access/updates
 	observersMu *sync.RWMutex
 	// observers is a list of channelObservers that will be notified when publishCh
-	// receives a value.
+	// receives a new value.
 	observers []*channelObserver[V]
 }
 
@@ -60,7 +60,7 @@ func WithProducer[V any](producer chan V) option[V] {
 // Subscribe returns an observer which is notified when the publishCh channel
 // receives a value.
 func (obsvbl *channelObservable[V]) Subscribe(ctx context.Context) observable.Observer[V] {
-	// must lock observersMu so that we can safely append to the observers list
+	// must (write) lock observersMu so that we can safely append to the observers list
 	obsvbl.observersMu.Lock()
 	defer obsvbl.observersMu.Unlock()
 
@@ -145,8 +145,8 @@ func (obsvbl *channelObservable[V]) copyObservers() (observers []*channelObserve
 	return observers
 }
 
-// goUnsubscribeOnDone unsubscribes from the subscription when the context is.
-// It is blocking and intended to be called in a goroutine.
+// goUnsubscribeOnDone unsubscribes from the subscription when the context is done.
+// It is a blocking function and intended to be called in a goroutine.
 func goUnsubscribeOnDone[V any](ctx context.Context, subscription observable.Observer[V]) {
 	<-ctx.Done()
 	subscription.Unsubscribe()
@@ -155,7 +155,7 @@ func goUnsubscribeOnDone[V any](ctx context.Context, subscription observable.Obs
 // onUnsubscribe returns a function that removes a given channelObserver from the
 // observable's list of observers.
 func (obsvbl *channelObservable[V]) onUnsubscribe(toRemove *channelObserver[V]) {
-	// must lock to iterato over and modify observers list
+	// must (write) lock to iterate over and modify the observers list
 	obsvbl.observersMu.Lock()
 	defer obsvbl.observersMu.Unlock()
 
diff --git a/pkg/observable/channel/observable_test.go b/pkg/observable/channel/observable_test.go
index 22167b806..840e6c0e0 100644
--- a/pkg/observable/channel/observable_test.go
+++ b/pkg/observable/channel/observable_test.go
@@ -32,7 +32,7 @@ func TestChannelObservable_NotifyObservers(t *testing.T) {
 
 	inputs := []int{123, 456, 789}
 	fullBufferedPublisher := make(chan *int, 1)
-	// NB: see INCOMPLETE comment below
+	// NB: see TODO_INCOMPLETE comment below
 	//fullBlockingPublisher := make(chan *int)
 
 	tests := []test{
@@ -108,7 +108,7 @@ func TestChannelObservable_NotifyObservers(t *testing.T) {
 
 			group, ctx := errgroup.WithContext(ctx)
 
-			// ensure all obsvr channels are notified
+			// ensure all obsvr channels are notified by starting a lot of asynchronous listeners error/success listeners
 			for obsvrIdx, obsvr := range observers {
 				next := func(outputIndex int, output *int) error {
 					// obsvr channel should receive notified input
@@ -237,6 +237,7 @@ func TestChannelObservable_UnsubscribeObservers(t *testing.T) {
 	}
 }
 
+// TODO_INCOMPLETE/TODO_TECHDEBT: Implement `TestChannelObservable_ConcurrentSubUnSub`
 func TestChannelObservable_ConcurrentSubUnSub(t *testing.T) {
 	t.Skip("add coverage: subscribing and unsubscribing concurrently should not race")
 }
diff --git a/pkg/observable/channel/observer.go b/pkg/observable/channel/observer.go
index 37c6f3071..d2623021b 100644
--- a/pkg/observable/channel/observer.go
+++ b/pkg/observable/channel/observer.go
@@ -83,13 +83,15 @@ func (obsvr *channelObserver[V]) unsubscribe() {
 	obsvr.onUnsubscribe(obsvr)
 }
 
-// notify is used called by observable to send on the observer channel. Can't
-// use channelObserver#Ch because it's receive-only. It will block if the channel
-// is full but will release the read-lock for half of the sendRetryInterval. The
-// other half holds is spent holding the read-lock and waiting for the (full)
-// channel to be ready to receive.
+// notify is called by observable to send a msg on the observer's channel.
+// We can't use channelObserver#Ch because it's intended to be a
+// receive-only channel. The channel will block if it is full (determined by the buffer
+// size)
+// if the channel's buffer is full, we will retry after sendRetryInterval/s.
+// The other half is spent holding the read-lock and waiting for the (full) channel
+// to be ready to receive.
 func (obsvr *channelObserver[V]) notify(value V) {
-	defer obsvr.observerMu.RUnlock()
+	defer obsvr.observerMu.RUnlock() // defer releasing a read lock
 
 	sendRetryTicker := time.NewTicker(sendRetryInterval / 2)
 	for {
@@ -109,6 +111,7 @@ func (obsvr *channelObserver[V]) notify(value V) {
 			// if the context is done just release the read-lock (deferred)
 			return
 		case obsvr.observerCh <- value:
+			// if observerCh has space in its buffer, the value is written to it
 			return
 		// if the context isn't done and channel is full (i.e. blocking),
 		// release the read-lock to give write-lockers a turn. This case
diff --git a/pkg/observable/channel/observer_test.go b/pkg/observable/channel/observer_test.go
index 83f05b94e..1d10afb57 100644
--- a/pkg/observable/channel/observer_test.go
+++ b/pkg/observable/channel/observer_test.go
@@ -16,7 +16,7 @@ func TestObserver_Unsubscribe(t *testing.T) {
 	)
 	obsvr := &channelObserver[int]{
 		observerMu: &sync.RWMutex{},
-		// using a buffered  channel to keep the test synchronous
+		// using a buffered channel to keep the test synchronous
 		observerCh: inputCh,
 		onUnsubscribe: func(toRemove *channelObserver[int]) {
 			onUnsubscribeCalled = true
@@ -43,7 +43,7 @@ func TestObserver_ConcurrentUnsubscribe(t *testing.T) {
 	obsvr := &channelObserver[int]{
 		ctx:        context.Background(),
 		observerMu: &sync.RWMutex{},
-		// using a buffered  channel to keep the test synchronous
+		// using a buffered channel to keep the test synchronous
 		observerCh: inputCh,
 		onUnsubscribe: func(toRemove *channelObserver[int]) {
 			onUnsubscribeCalled = true
diff --git a/pkg/observable/interface.go b/pkg/observable/interface.go
index 3b224c889..c680d8f1e 100644
--- a/pkg/observable/interface.go
+++ b/pkg/observable/interface.go
@@ -1,5 +1,7 @@
 package observable
 
+// TODO(@bryanchriswhite): Add a README for the observable.
+
 import "context"
 
 // NOTE: We explicitly decided to write a small and custom notifications package
@@ -9,13 +11,15 @@ import "context"
 
 // Observable is a generic interface that allows multiple subscribers to be
 // notified of new values asynchronously.
+// It is analogous to a publisher in a "Fan-Out" system design.
 type Observable[V any] interface {
 	Subscribe(context.Context) Observer[V]
 	Close()
 }
 
 // Observer is a generic interface that provides access to the notified
-// channel and allows unsubscribing from an observable.
+// channel and allows unsubscribing from an Observable.
+// It is analogous to a subscriber in a "Fan-Out" system design.
 type Observer[V any] interface {
 	Unsubscribe()
 	Ch() <-chan V

From c449f14bf99b701c775c51db2d87c6d21e14f48f Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Thu, 19 Oct 2023 19:54:50 +0200
Subject: [PATCH 30/41] chore: improve comments

Co-authored-by: Daniel Olshansky <olshansky.daniel@gmail.com>
---
 pkg/observable/channel/observer.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/pkg/observable/channel/observer.go b/pkg/observable/channel/observer.go
index d2623021b..fc797774e 100644
--- a/pkg/observable/channel/observer.go
+++ b/pkg/observable/channel/observer.go
@@ -117,10 +117,10 @@ func (obsvr *channelObserver[V]) notify(value V) {
 		// release the read-lock to give write-lockers a turn. This case
 		// continues the loop, re-read-locking and trying again.
 		case <-sendRetryTicker.C:
-			// CONSIDERATION: repurpose this retry loop into a default path which
+			// TODO_CONSIDERATION: repurpose this retry loop into a default path which
 			// buffers values so that the publishCh doesn't block and other observers
 			// can still be notified.
-			// TECHDEBT: add some logic to drain the buffer at some appropriate time
+			// TODO_TECHDEBT: add some logic to periodically drain the buffer at some appropriate time
 
 			// this case implies that the (read) lock was acquired, so it must
 			// be unlocked before continuing the send retry loop.

From b97e6e24845a984c5b194fc1677a8223c8177ee0 Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Thu, 19 Oct 2023 19:52:26 +0200
Subject: [PATCH 31/41] refactor: rename `Observable#Close()` to
 `#UnsubscribeAll()`

---
 pkg/observable/channel/observable.go      | 16 ++++++++--------
 pkg/observable/channel/observable_test.go |  4 ++--
 pkg/observable/interface.go               |  2 +-
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/pkg/observable/channel/observable.go b/pkg/observable/channel/observable.go
index bec168abd..8d1cd321a 100644
--- a/pkg/observable/channel/observable.go
+++ b/pkg/observable/channel/observable.go
@@ -67,22 +67,22 @@ func (obsvbl *channelObservable[V]) Subscribe(ctx context.Context) observable.Ob
 	observer := NewObserver[V](ctx, obsvbl.onUnsubscribe)
 	obsvbl.observers = append(obsvbl.observers, observer)
 
-	// caller can rely on context cancellation or call Close() to unsubscribe
+	// caller can rely on context cancellation or call UnsubscribeAll() to unsubscribe
 	// active observers
 	if ctx != nil {
-		// asynchronously wait for the context to close and unsubscribe
+		// asynchronously wait for the context to unsubscribeAll and unsubscribe
 		go goUnsubscribeOnDone[V](ctx, observer)
 	}
 	return observer
 }
 
-func (obsvbl *channelObservable[V]) Close() {
-	obsvbl.close()
+// UnsubscribeAll unsubscribes and removes all observers from the observable.
+func (obsvbl *channelObservable[V]) UnsubscribeAll() {
+	obsvbl.unsubscribeAll()
 }
 
-// TODO_CONSIDERATION: decide whether this should close the publishCh channel; perhaps
-// only if it was provided.
-func (obsvbl *channelObservable[V]) close() {
+// unsubscribeAll unsubscribes and removes all observers from the observable.
+func (obsvbl *channelObservable[V]) unsubscribeAll() {
 	// Copy currentObservers to avoid holding the lock while unsubscribing them.
 	// The current observers at this time is the canonical set of observers which
 	// will be unsubscribed.
@@ -123,7 +123,7 @@ func (obsvbl *channelObservable[V]) goProduce(publisher <-chan V) {
 	}
 
 	// Here we know that the publishCh has been isClosed, all currentObservers should be isClosed as well
-	obsvbl.close()
+	obsvbl.unsubscribeAll()
 }
 
 func (obsvbl *channelObservable[V]) copyObservers() (observers []*channelObserver[V]) {
diff --git a/pkg/observable/channel/observable_test.go b/pkg/observable/channel/observable_test.go
index 840e6c0e0..fb9e5d0b0 100644
--- a/pkg/observable/channel/observable_test.go
+++ b/pkg/observable/channel/observable_test.go
@@ -152,7 +152,7 @@ func TestChannelObservable_NotifyObservers(t *testing.T) {
 			err := group.Wait()
 			require.NoError(t, err)
 
-			// unsubscribing should close obsvr channel(s)
+			// unsubscribing should unsubscribeAll obsvr channel(s)
 			for _, observer := range observers {
 				observer.Unsubscribe()
 
@@ -323,7 +323,7 @@ func TestChannelObservable_SequentialProductionAndUnsubscription(t *testing.T) {
 
 // TECHDEBT/INCOMPLETE: add coverage for active observers closing when publishCh closes.
 func TestChannelObservable_ObserversCloseOnProducerClose(t *testing.T) {
-	t.Skip("add coverage: all observers should close when publishCh closes")
+	t.Skip("add coverage: all observers should unsubscribeAll when publishCh closes")
 }
 
 func produceWithDelay[V any](producer chan<- V, delay time.Duration) func(value V) {
diff --git a/pkg/observable/interface.go b/pkg/observable/interface.go
index c680d8f1e..1e151f4b3 100644
--- a/pkg/observable/interface.go
+++ b/pkg/observable/interface.go
@@ -14,7 +14,7 @@ import "context"
 // It is analogous to a publisher in a "Fan-Out" system design.
 type Observable[V any] interface {
 	Subscribe(context.Context) Observer[V]
-	Close()
+	UnsubscribeAll()
 }
 
 // Observer is a generic interface that provides access to the notified

From bb6055e55370cdd7534330a22a5a632cb586e5f9 Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Thu, 19 Oct 2023 19:52:58 +0200
Subject: [PATCH 32/41] chore: improve comments

---
 pkg/observable/channel/observer.go      | 10 +++++-----
 pkg/observable/channel/observer_test.go |  1 -
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/pkg/observable/channel/observer.go b/pkg/observable/channel/observer.go
index fc797774e..8a8dc3291 100644
--- a/pkg/observable/channel/observer.go
+++ b/pkg/observable/channel/observer.go
@@ -117,12 +117,12 @@ func (obsvr *channelObserver[V]) notify(value V) {
 		// release the read-lock to give write-lockers a turn. This case
 		// continues the loop, re-read-locking and trying again.
 		case <-sendRetryTicker.C:
-			// TODO_CONSIDERATION: repurpose this retry loop into a default path which
-			// buffers values so that the publishCh doesn't block and other observers
-			// can still be notified.
-			// TODO_TECHDEBT: add some logic to periodically drain the buffer at some appropriate time
+			// TODO_IMPROVE/TODO_CONSIDERATION: this is where we would implement
+			// some backpressure strategy. It would be good to have a simple fail-
+			// safe strategy that can be used by default; e.g. dropping the oldest
+			// value if its buffer is full.
 
-			// this case implies that the (read) lock was acquired, so it must
+			// This case implies that the (read) lock was acquired, so it must
 			// be unlocked before continuing the send retry loop.
 			obsvr.observerMu.RUnlock()
 		}
diff --git a/pkg/observable/channel/observer_test.go b/pkg/observable/channel/observer_test.go
index 1d10afb57..9b99e72f4 100644
--- a/pkg/observable/channel/observer_test.go
+++ b/pkg/observable/channel/observer_test.go
@@ -63,7 +63,6 @@ func TestObserver_ConcurrentUnsubscribe(t *testing.T) {
 			}
 
 			obsvr.notify(inputIdx)
-			//time.Sleep(50 * time.Millisecond)
 		}
 	}()
 	t.Cleanup(func() { done <- struct{}{} })

From 70b3e5940b62d60957cd2b1c9e50f00a9fce91bd Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Thu, 19 Oct 2023 20:43:22 +0200
Subject: [PATCH 33/41] chore: misc. review feedback improvements

---
 pkg/observable/channel/observable.go       |  27 ++++--
 pkg/observable/channel/observable_test.go  | 108 +++++++++++----------
 pkg/observable/channel/observation_test.go |  27 ++++--
 pkg/observable/channel/observer.go         |  15 ++-
 pkg/observable/channel/observer_test.go    |  17 ++--
 pkg/observable/errors.go                   |   8 ++
 6 files changed, 123 insertions(+), 79 deletions(-)
 create mode 100644 pkg/observable/errors.go

diff --git a/pkg/observable/channel/observable.go b/pkg/observable/channel/observable.go
index 8d1cd321a..fc7ebbb7f 100644
--- a/pkg/observable/channel/observable.go
+++ b/pkg/observable/channel/observable.go
@@ -6,6 +6,12 @@ import (
 	"sync"
 )
 
+// TODO_DISCUSS: what should this be? should it be configurable? It seems to be most
+// relevant in the context of the behavior of the observable when it has multiple
+// observers which consume at different rates.
+// defaultSubscribeBufferSize is the buffer size of a channelObserver's channel.
+const defaultPublishBufferSize = 50
+
 var _ observable.Observable[any] = &channelObservable[any]{}
 
 // option is a function which receives and can modify the channelObservable state.
@@ -40,20 +46,20 @@ func NewObservable[V any](opts ...option[V]) (observable.Observable[V], chan<- V
 
 	// if the caller does not provide a publishCh, create a new one and return it
 	if obs.publishCh == nil {
-		obs.publishCh = make(chan V)
+		obs.publishCh = make(chan V, defaultPublishBufferSize)
 	}
 
 	// start listening to the publishCh and emit values to observers
-	go obs.goProduce(obs.publishCh)
+	go obs.goPublish(obs.publishCh)
 
 	return obs, obs.publishCh
 }
 
-// WithProducer returns an option function which sets the given publishCh of the
+// WithPublisher returns an option function which sets the given publishCh of the
 // resulting observable when passed to NewObservable().
-func WithProducer[V any](producer chan V) option[V] {
+func WithPublisher[V any](publishCh chan V) option[V] {
 	return func(obs *channelObservable[V]) {
-		obs.publishCh = producer
+		obs.publishCh = publishCh
 	}
 }
 
@@ -102,9 +108,9 @@ func (obsvbl *channelObservable[V]) unsubscribeAll() {
 	obsvbl.observersMu.Unlock()
 }
 
-// goProduce to the publishCh and notify observers when values are received. This
-// function is blocking and should be run in a goroutine.
-func (obsvbl *channelObservable[V]) goProduce(publisher <-chan V) {
+// goPublish to the publishCh and notify observers when values are received.
+// This function is blocking and should be run in a goroutine.
+func (obsvbl *channelObservable[V]) goPublish(publisher <-chan V) {
 	for notification := range publisher {
 		// Copy currentObservers to avoid holding the lock while notifying them.
 		// New or existing Observers may (un)subscribe while this notification
@@ -122,10 +128,13 @@ func (obsvbl *channelObservable[V]) goProduce(publisher <-chan V) {
 		}
 	}
 
-	// Here we know that the publishCh has been isClosed, all currentObservers should be isClosed as well
+	// Here we know that the publisher channel has been closed.
+	// Unsubscribe all observers as they can no longer receive notifications.
 	obsvbl.unsubscribeAll()
 }
 
+// copyObservers returns a copy of the current observers list. It is safe to
+// call concurrently.
 func (obsvbl *channelObservable[V]) copyObservers() (observers []*channelObserver[V]) {
 	defer obsvbl.observersMu.RUnlock()
 
diff --git a/pkg/observable/channel/observable_test.go b/pkg/observable/channel/observable_test.go
index fb9e5d0b0..c4289bbc1 100644
--- a/pkg/observable/channel/observable_test.go
+++ b/pkg/observable/channel/observable_test.go
@@ -16,24 +16,24 @@ import (
 )
 
 const (
-	productionDelay        = 2 * time.Millisecond
-	notifyTimeout          = productionDelay * 5
+	publishDelay           = 2 * time.Millisecond
+	notifyTimeout          = publishDelay * 5
 	cancelUnsubscribeDelay = notifyTimeout * 2
 )
 
 func TestChannelObservable_NotifyObservers(t *testing.T) {
 	type test struct {
 		name            string
-		publishCh       chan *int
+		publishCh       chan int
 		inputs          []int
 		expectedOutputs []int
 		setupFn         func(t test)
 	}
 
 	inputs := []int{123, 456, 789}
-	fullBufferedPublisher := make(chan *int, 1)
 	// NB: see TODO_INCOMPLETE comment below
 	//fullBlockingPublisher := make(chan *int)
+	//fullBufferedPublisher := make(chan *int, 1)
 
 	tests := []test{
 		{
@@ -44,26 +44,16 @@ func TestChannelObservable_NotifyObservers(t *testing.T) {
 		},
 		{
 			name:            "empty non-buffered publisher",
-			publishCh:       make(chan *int),
+			publishCh:       make(chan int),
 			inputs:          inputs,
 			expectedOutputs: inputs,
 		},
 		{
 			name:            "empty buffered len 1 publisher",
-			publishCh:       make(chan *int, 1),
+			publishCh:       make(chan int, 1),
 			inputs:          inputs,
 			expectedOutputs: inputs,
 		},
-		{
-			name:            "full buffered len 1 publisher",
-			publishCh:       fullBufferedPublisher,
-			inputs:          inputs[1:],
-			expectedOutputs: inputs,
-			setupFn: func(t test) {
-				// non-blocking send
-				t.publishCh <- &inputs[0]
-			},
-		},
 		// INCOMPLETE: publisher channels which are full are proving harder to test
 		// robustly (no flakiness); perhaps it has to do with the lack of some
 		// kind of guarantee about the receiver order on the consumer side.
@@ -82,6 +72,16 @@ func TestChannelObservable_NotifyObservers(t *testing.T) {
 		// 		}()
 		// 	},
 		// },
+		// {
+		// 	name:            "full buffered len 1 publisher",
+		// 	publishCh:       fullBufferedPublisher,
+		// 	inputs:          inputs[1:],
+		// 	expectedOutputs: inputs,
+		// 	setupFn: func(t test) {
+		// 		// non-blocking send
+		// 		t.publishCh <- &inputs[0]
+		// 	},
+		// },
 	}
 
 	for _, tt := range tests {
@@ -93,28 +93,28 @@ func TestChannelObservable_NotifyObservers(t *testing.T) {
 			ctx, cancel := context.WithCancel(context.Background())
 			t.Cleanup(cancel)
 
-			obsvbl, producer := channel.NewObservable[*int](
-				channel.WithProducer(tt.publishCh),
+			obsvbl, publisher := channel.NewObservable[int](
+				channel.WithPublisher(tt.publishCh),
 			)
 			require.NotNil(t, obsvbl)
-			require.NotNil(t, producer)
-			produce := produceWithDelay(producer, productionDelay)
+			require.NotNil(t, publisher)
 
 			// construct 3 distinct observers, each with its own channel
-			observers := make([]observable.Observer[*int], 1)
+			observers := make([]observable.Observer[int], 1)
 			for i := range observers {
 				observers[i] = obsvbl.Subscribe(ctx)
 			}
 
 			group, ctx := errgroup.WithContext(ctx)
 
-			// ensure all obsvr channels are notified by starting a lot of asynchronous listeners error/success listeners
+			// ensure all observer channels are notified
 			for obsvrIdx, obsvr := range observers {
-				next := func(outputIndex int, output *int) error {
+				// onNext is called for each notification received by the observer
+				onNext := func(outputIndex int, output int) error {
 					// obsvr channel should receive notified input
 					if !assert.Equalf(
 						t, tt.expectedOutputs[outputIndex],
-						*output,
+						output,
 						"obsvr Idx: %d", obsvrIdx,
 					) {
 						return fmt.Errorf("unexpected output")
@@ -122,7 +122,8 @@ func TestChannelObservable_NotifyObservers(t *testing.T) {
 					return nil
 				}
 
-				done := func(outputs []*int) error {
+				// onDone is called when the observer channel closes
+				onDone := func(outputs []int) error {
 					if !assert.Equalf(
 						t, len(tt.expectedOutputs),
 						len(outputs),
@@ -134,17 +135,18 @@ func TestChannelObservable_NotifyObservers(t *testing.T) {
 				}
 
 				// concurrently await notification or timeout to avoid blocking on
-				// empty and/or non-buffered producers.
-				group.Go(goNotifiedOrTimedOutFactory(obsvr, next, done, notifyTimeout))
+				// empty and/or non-buffered publishers.
+				group.Go(goNotifiedOrTimedOutFactory(obsvr, onNext, onDone, notifyTimeout))
 			}
 
 			// notify with test input
+			publish := delayedPublishFactory(publisher, publishDelay)
 			for _, input := range tt.inputs {
 				inputPtr := new(int)
 				*inputPtr = input
 
-				// simulating IO delay in sequential message production
-				produce(inputPtr)
+				// simulating IO delay in sequential message publishing
+				publish(input)
 			}
 			cancel()
 
@@ -166,9 +168,9 @@ func TestChannelObservable_NotifyObservers(t *testing.T) {
 
 func TestChannelObservable_UnsubscribeObservers(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
-	obsvbl, producer := channel.NewObservable[int]()
+	obsvbl, publishCh := channel.NewObservable[int]()
 	require.NotNil(t, obsvbl)
-	require.NotNil(t, producer)
+	require.NotNil(t, publishCh)
 
 	type test struct {
 		name        string
@@ -242,7 +244,7 @@ func TestChannelObservable_ConcurrentSubUnSub(t *testing.T) {
 	t.Skip("add coverage: subscribing and unsubscribing concurrently should not race")
 }
 
-func TestChannelObservable_SequentialProductionAndUnsubscription(t *testing.T) {
+func TestChannelObservable_SequentialPublishAndUnsubscription(t *testing.T) {
 	observations := new([]*observation[int])
 	expectedNotifications := [][]int{
 		{123, 456, 789},
@@ -251,11 +253,11 @@ func TestChannelObservable_SequentialProductionAndUnsubscription(t *testing.T) {
 		{987, 654, 321},
 	}
 
-	obsvbl, producer := channel.NewObservable[int]()
+	obsvbl, publishCh := channel.NewObservable[int]()
 	require.NotNil(t, obsvbl)
-	require.NotNil(t, producer)
-	// simulate IO delay in sequential message production
-	produceWithDelay := produceWithDelay(producer, productionDelay)
+	require.NotNil(t, publishCh)
+	// simulate IO delay in sequential message publishing
+	publish := delayedPublishFactory(publishCh, publishDelay)
 
 	ctx, cancel := context.WithCancel(context.Background())
 	t.Cleanup(cancel)
@@ -263,30 +265,30 @@ func TestChannelObservable_SequentialProductionAndUnsubscription(t *testing.T) {
 	observation0 := newObservation(ctx, obsvbl)
 	*observations = append(*observations, observation0)
 	go goReceiveNotifications(observation0)
-	produceWithDelay(123)
+	publish(123)
 
 	observation1 := newObservation(ctx, obsvbl)
 	*observations = append(*observations, observation1)
 	go goReceiveNotifications(observation1)
-	produceWithDelay(456)
+	publish(456)
 
 	observation2 := newObservation(ctx, obsvbl)
 	*observations = append(*observations, observation2)
 	go goReceiveNotifications(observation2)
-	produceWithDelay(789)
+	publish(789)
 
 	observation3 := newObservation(ctx, obsvbl)
 	*observations = append(*observations, observation3)
 	go goReceiveNotifications(observation3)
 
 	observation0.Unsubscribe()
-	produceWithDelay(987)
+	publish(987)
 
 	observation1.Unsubscribe()
-	produceWithDelay(654)
+	publish(654)
 
 	observation2.Unsubscribe()
-	produceWithDelay(321)
+	publish(321)
 
 	observation3.Unsubscribe()
 
@@ -305,16 +307,16 @@ func TestChannelObservable_SequentialProductionAndUnsubscription(t *testing.T) {
 
 			require.Equalf(
 				t, len(expectedNotifications[obsnIdx]),
-				len(*obsrvn.Notifications),
+				len(obsrvn.Notifications),
 				"observation index: %d, expected: %+v, actual: %+v",
-				obsnIdx, expectedNotifications[obsnIdx], *obsrvn.Notifications,
+				obsnIdx, expectedNotifications[obsnIdx], obsrvn.Notifications,
 			)
 			for notificationIdx, expected := range expectedNotifications[obsnIdx] {
 				require.Equalf(
 					t, expected,
-					(*obsrvn.Notifications)[notificationIdx],
+					(obsrvn.Notifications)[notificationIdx],
 					"allExpected: %+v, allActual: %+v",
-					expectedNotifications[obsnIdx], *obsrvn.Notifications,
+					expectedNotifications[obsnIdx], obsrvn.Notifications,
 				)
 			}
 		})
@@ -322,22 +324,22 @@ func TestChannelObservable_SequentialProductionAndUnsubscription(t *testing.T) {
 }
 
 // TECHDEBT/INCOMPLETE: add coverage for active observers closing when publishCh closes.
-func TestChannelObservable_ObserversCloseOnProducerClose(t *testing.T) {
+func TestChannelObservable_ObserversCloseOnPublishChannelClose(t *testing.T) {
 	t.Skip("add coverage: all observers should unsubscribeAll when publishCh closes")
 }
 
-func produceWithDelay[V any](producer chan<- V, delay time.Duration) func(value V) {
+func delayedPublishFactory[V any](publishCh chan<- V, delay time.Duration) func(value V) {
 	return func(value V) {
 		time.Sleep(delay / 2)
-		producer <- value
+		publishCh <- value
 		time.Sleep(delay / 2)
 	}
 }
 
 func goNotifiedOrTimedOutFactory[V any](
 	obsvr observable.Observer[V],
-	next func(index int, output V) error,
-	done func(outputs []V) error,
+	onNext func(index int, output V) error,
+	onDone func(outputs []V) error,
 	timeoutDuration time.Duration,
 ) func() error {
 	var (
@@ -349,10 +351,10 @@ func goNotifiedOrTimedOutFactory[V any](
 			select {
 			case output, ok := <-obsvr.Ch():
 				if !ok {
-					return done(outputs)
+					return onDone(outputs)
 				}
 
-				if err := next(outputIndex, output); err != nil {
+				if err := onNext(outputIndex, output); err != nil {
 					return err
 				}
 
diff --git a/pkg/observable/channel/observation_test.go b/pkg/observable/channel/observation_test.go
index a6bfa41fc..1402549ab 100644
--- a/pkg/observable/channel/observation_test.go
+++ b/pkg/observable/channel/observation_test.go
@@ -7,31 +7,44 @@ import (
 	"pocket/pkg/observable"
 )
 
+// observation is a data structure that embeds an observer
+// and keeps track of the received notifications.
+// It uses generics with type parameter V.
 type observation[V any] struct {
+	// Embeds a mutex for thread-safe operations
 	sync.Mutex
+	// Embeds an Observer of type V
 	observable.Observer[V]
-	Notifications *[]V
+	// Notifications is a slice of type V to store received notifications
+	Notifications []V
 }
 
+// newObservation is a constructor function that returns
+// a new observation instance. It subscribes to the provided observable.
 func newObservation[V any](
 	ctx context.Context,
 	observable observable.Observable[V],
 ) *observation[V] {
 	return &observation[V]{
 		Observer:      observable.Subscribe(ctx),
-		Notifications: new([]V),
+		Notifications: []V{},
 	}
 }
 
+// notify is a method on observation that safely
+// appends a received value to the Notifications slice.
 func (o *observation[V]) notify(value V) {
-	o.Lock()
-	defer o.Unlock()
+	o.Lock()         // Locks the mutex to prevent concurrent write access
+	defer o.Unlock() // Unlocks the mutex when the method returns
 
-	*o.Notifications = append(*o.Notifications, value)
+	o.Notifications = append(o.Notifications, value) // Appends the received value to the Notifications slice
 }
 
+// goReceiveNotifications is a function that listens for
+// notifications from the observer's channel and notifies
+// the observation instance for each received value.
 func goReceiveNotifications[V any](obsvn *observation[V]) {
-	for notification := range obsvn.Ch() {
-		obsvn.notify(notification)
+	for notification := range obsvn.Ch() { // Listens for notifications on the channel
+		obsvn.notify(notification) // Notifies the observation instance with the received value
 	}
 }
diff --git a/pkg/observable/channel/observer.go b/pkg/observable/channel/observer.go
index 8a8dc3291..d45fd56c0 100644
--- a/pkg/observable/channel/observer.go
+++ b/pkg/observable/channel/observer.go
@@ -2,6 +2,7 @@ package channel
 
 import (
 	"context"
+	"log"
 	"sync"
 	"time"
 
@@ -12,8 +13,8 @@ const (
 	// TODO_DISCUSS: what should this be? should it be configurable? It seems to be most
 	// relevant in the context of the behavior of the observable when it has multiple
 	// observers which consume at different rates.
-	// observerBufferSize is the buffer size of a channelObserver's channel.
-	observerBufferSize = 1
+	// defaultSubscribeBufferSize is the buffer size of a channelObserver's channel.
+	defaultSubscribeBufferSize = 50
 	// sendRetryInterval is the duration between attempts to send on the observer's
 	// channel in the event that it's full. It facilitates a branch in a for loop
 	// which unlocks the observer's mutex and tries again.
@@ -52,7 +53,7 @@ func NewObserver[V any](
 	return &channelObserver[V]{
 		ctx:           ctx,
 		observerMu:    new(sync.RWMutex),
-		observerCh:    make(chan V, observerBufferSize),
+		observerCh:    make(chan V, defaultSubscribeBufferSize),
 		onUnsubscribe: onUnsubscribe,
 	}
 }
@@ -75,6 +76,14 @@ func (obsvr *channelObserver[V]) unsubscribe() {
 	defer obsvr.observerMu.Unlock()
 
 	if obsvr.isClosed {
+		// log the fact that this case was encountered such that an extreme change
+		// in its frequency would be obvious.
+		// TODO_TECHDEBT: integrate with structured logger once available
+		// TODO_CONSIDERATION: alternative perspective:
+		//   1. this is library code; prefer fewer external dependencies, esp. I/O
+		//   2. the stdlib log pkg is pretty good, idiomatic, and globally
+		//      configurable; perhaps it is sufficient
+		log.Printf("%s", observable.ErrObserverClosed.Wrap("redundant unsubscribe"))
 		return
 	}
 
diff --git a/pkg/observable/channel/observer_test.go b/pkg/observable/channel/observer_test.go
index 9b99e72f4..c115061bb 100644
--- a/pkg/observable/channel/observer_test.go
+++ b/pkg/observable/channel/observer_test.go
@@ -38,37 +38,40 @@ func TestObserver_Unsubscribe(t *testing.T) {
 func TestObserver_ConcurrentUnsubscribe(t *testing.T) {
 	var (
 		onUnsubscribeCalled = false
-		inputCh             = make(chan int, 1)
+		publishCh           = make(chan int, 1)
 	)
 	obsvr := &channelObserver[int]{
 		ctx:        context.Background(),
 		observerMu: &sync.RWMutex{},
 		// using a buffered channel to keep the test synchronous
-		observerCh: inputCh,
+		observerCh: publishCh,
 		onUnsubscribe: func(toRemove *channelObserver[int]) {
 			onUnsubscribeCalled = true
 		},
 	}
 
-	// should initially be open
-	require.Equal(t, false, obsvr.isClosed)
+	require.Equal(t, false, obsvr.isClosed, "observer channel should initially be open")
 
+	// publish until the test cleanup runs
 	done := make(chan struct{}, 1)
 	go func() {
-		for inputIdx := 0; ; inputIdx++ {
+		for idx := 0; ; idx++ {
+			// return when done receives; otherwise,
 			select {
 			case <-done:
 				return
 			default:
 			}
 
-			obsvr.notify(inputIdx)
+			// publish a value
+			obsvr.notify(idx)
 		}
 	}()
+	// send on done when the test cleans up
 	t.Cleanup(func() { done <- struct{}{} })
 
 	// wait a bit, then assert that the observer is still open
-	time.Sleep(50 * time.Millisecond)
+	time.Sleep(10 * time.Millisecond)
 
 	require.Equal(t, false, obsvr.isClosed)
 
diff --git a/pkg/observable/errors.go b/pkg/observable/errors.go
new file mode 100644
index 000000000..c34f2f7d6
--- /dev/null
+++ b/pkg/observable/errors.go
@@ -0,0 +1,8 @@
+package observable
+
+import errorsmod "cosmossdk.io/errors"
+
+var (
+	ErrObserverClosed = errorsmod.Register(codespace, 1, "observer is closed")
+	codespace         = "observable"
+)

From 26f1d1350a10e8ab002e335aa9609afa1dc037c4 Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Thu, 19 Oct 2023 20:45:09 +0200
Subject: [PATCH 34/41] chore: improve comment

---
 pkg/observable/channel/observation_test.go | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/pkg/observable/channel/observation_test.go b/pkg/observable/channel/observation_test.go
index 1402549ab..17e20e393 100644
--- a/pkg/observable/channel/observation_test.go
+++ b/pkg/observable/channel/observation_test.go
@@ -7,6 +7,8 @@ import (
 	"pocket/pkg/observable"
 )
 
+// NOTE: this file does not contain any tests, only test helpers.
+
 // observation is a data structure that embeds an observer
 // and keeps track of the received notifications.
 // It uses generics with type parameter V.

From c6fe88abd06b2797c9ddbcb8067d793677f69021 Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Thu, 19 Oct 2023 21:40:39 +0200
Subject: [PATCH 35/41] chore: review improvements

---
 pkg/observable/channel/observable_test.go |  6 ++++--
 pkg/observable/channel/observer.go        |  4 +---
 pkg/observable/channel/observer_test.go   | 11 +++++------
 3 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/pkg/observable/channel/observable_test.go b/pkg/observable/channel/observable_test.go
index c4289bbc1..48f7ba7c4 100644
--- a/pkg/observable/channel/observable_test.go
+++ b/pkg/observable/channel/observable_test.go
@@ -330,9 +330,11 @@ func TestChannelObservable_ObserversCloseOnPublishChannelClose(t *testing.T) {
 
 func delayedPublishFactory[V any](publishCh chan<- V, delay time.Duration) func(value V) {
 	return func(value V) {
-		time.Sleep(delay / 2)
 		publishCh <- value
-		time.Sleep(delay / 2)
+		// simulate IO delay in sequential message publishing
+		// NB: this make the test code safer as concurrent operations have more
+		// time to react; i.e. interact with the test harness.
+		time.Sleep(delay)
 	}
 }
 
diff --git a/pkg/observable/channel/observer.go b/pkg/observable/channel/observer.go
index d45fd56c0..7bd7ddbd3 100644
--- a/pkg/observable/channel/observer.go
+++ b/pkg/observable/channel/observer.go
@@ -102,13 +102,12 @@ func (obsvr *channelObserver[V]) unsubscribe() {
 func (obsvr *channelObserver[V]) notify(value V) {
 	defer obsvr.observerMu.RUnlock() // defer releasing a read lock
 
-	sendRetryTicker := time.NewTicker(sendRetryInterval / 2)
+	sendRetryTicker := time.NewTicker(sendRetryInterval)
 	for {
 		// observerMu must remain read-locked until the value is sent on observerCh
 		// in the event that it would be isClosed concurrently (i.e. this observer
 		// unsubscribes), which could cause a "send on isClosed channel" error.
 		if !obsvr.observerMu.TryRLock() {
-			time.Sleep(sendRetryInterval / 2)
 			continue
 		}
 		if obsvr.isClosed {
@@ -135,6 +134,5 @@ func (obsvr *channelObserver[V]) notify(value V) {
 			// be unlocked before continuing the send retry loop.
 			obsvr.observerMu.RUnlock()
 		}
-		time.Sleep(sendRetryInterval / 2)
 	}
 }
diff --git a/pkg/observable/channel/observer_test.go b/pkg/observable/channel/observer_test.go
index c115061bb..f8730a422 100644
--- a/pkg/observable/channel/observer_test.go
+++ b/pkg/observable/channel/observer_test.go
@@ -12,12 +12,12 @@ import (
 func TestObserver_Unsubscribe(t *testing.T) {
 	var (
 		onUnsubscribeCalled = false
-		inputCh             = make(chan int, 1)
+		publishCh           = make(chan int, 1)
 	)
 	obsvr := &channelObserver[int]{
 		observerMu: &sync.RWMutex{},
 		// using a buffered channel to keep the test synchronous
-		observerCh: inputCh,
+		observerCh: publishCh,
 		onUnsubscribe: func(toRemove *channelObserver[int]) {
 			onUnsubscribeCalled = true
 		},
@@ -26,7 +26,7 @@ func TestObserver_Unsubscribe(t *testing.T) {
 	// should initially be open
 	require.Equal(t, false, obsvr.isClosed)
 
-	inputCh <- 1
+	publishCh <- 1
 	require.Equal(t, false, obsvr.isClosed)
 
 	obsvr.Unsubscribe()
@@ -52,7 +52,7 @@ func TestObserver_ConcurrentUnsubscribe(t *testing.T) {
 
 	require.Equal(t, false, obsvr.isClosed, "observer channel should initially be open")
 
-	// publish until the test cleanup runs
+	// concurrently & continuously publish until the test cleanup runs
 	done := make(chan struct{}, 1)
 	go func() {
 		for idx := 0; ; idx++ {
@@ -70,9 +70,8 @@ func TestObserver_ConcurrentUnsubscribe(t *testing.T) {
 	// send on done when the test cleans up
 	t.Cleanup(func() { done <- struct{}{} })
 
-	// wait a bit, then assert that the observer is still open
+	// it should still be open after a bit of inactivity
 	time.Sleep(10 * time.Millisecond)
-
 	require.Equal(t, false, obsvr.isClosed)
 
 	obsvr.Unsubscribe()

From 835bad051c15d5bfee7a0a2aa80899e8fb93ded6 Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Thu, 19 Oct 2023 22:01:01 +0200
Subject: [PATCH 36/41] chore: last minute improvements

---
 internal/testchannel/drain.go             |  7 +++---
 pkg/observable/channel/observable.go      | 30 +++++++++++------------
 pkg/observable/channel/observable_test.go |  6 ++---
 3 files changed, 20 insertions(+), 23 deletions(-)

diff --git a/internal/testchannel/drain.go b/internal/testchannel/drain.go
index 82fee2ca3..4ea41a297 100644
--- a/internal/testchannel/drain.go
+++ b/internal/testchannel/drain.go
@@ -1,11 +1,10 @@
 package testchannel
 
 import (
-	"errors"
 	"time"
-)
 
-var errChanNotClosed = errors.New("channel is not closed")
+	"pocket/pkg/observable"
+)
 
 // DrainChannel attempts to receive from the given channel, blocking, until it is
 // empty. It returns an error if the channel is not closed by the time it's empty.
@@ -21,7 +20,7 @@ func DrainChannel[V any](ch <-chan V) error {
 			}
 			return nil
 		case <-time.After(time.Millisecond):
-			return errChanNotClosed
+			return observable.ErrObserverClosed
 		}
 	}
 }
diff --git a/pkg/observable/channel/observable.go b/pkg/observable/channel/observable.go
index fc7ebbb7f..a173d8072 100644
--- a/pkg/observable/channel/observable.go
+++ b/pkg/observable/channel/observable.go
@@ -9,7 +9,7 @@ import (
 // TODO_DISCUSS: what should this be? should it be configurable? It seems to be most
 // relevant in the context of the behavior of the observable when it has multiple
 // observers which consume at different rates.
-// defaultSubscribeBufferSize is the buffer size of a channelObserver's channel.
+// defaultSubscribeBufferSize is the buffer size of a observable's publish channel.
 const defaultPublishBufferSize = 50
 
 var _ observable.Observable[any] = &channelObservable[any]{}
@@ -18,7 +18,7 @@ var _ observable.Observable[any] = &channelObservable[any]{}
 type option[V any] func(obs *channelObservable[V])
 
 // channelObservable implements the observable.Observable interface and can be notified
-// via its corresponding publishCh channel.
+// by sending on its corresponding publishCh channel.
 type channelObservable[V any] struct {
 	// publishCh is an observable-wide channel that is used to receive values
 	// which are subsequently fanned out to observers.
@@ -30,9 +30,8 @@ type channelObservable[V any] struct {
 	observers []*channelObserver[V]
 }
 
-// NewObservable creates a new observable is notified when the publishCh channel
-// receives a value.
-// func NewObservable[V any](publishCh chan V) (observable.Observable[V], chan<- V) {
+// NewObservable creates a new observable which is notified when the publishCh
+// channel receives a value.
 func NewObservable[V any](opts ...option[V]) (observable.Observable[V], chan<- V) {
 	// initialize an observable that publishes messages from 1 publishCh to N observers
 	obs := &channelObservable[V]{
@@ -44,7 +43,8 @@ func NewObservable[V any](opts ...option[V]) (observable.Observable[V], chan<- V
 		opt(obs)
 	}
 
-	// if the caller does not provide a publishCh, create a new one and return it
+	// If the caller does not provide a publishCh, create a new one using the
+	// defaultPublishBuffer size and return it.
 	if obs.publishCh == nil {
 		obs.publishCh = make(chan V, defaultPublishBufferSize)
 	}
@@ -76,7 +76,8 @@ func (obsvbl *channelObservable[V]) Subscribe(ctx context.Context) observable.Ob
 	// caller can rely on context cancellation or call UnsubscribeAll() to unsubscribe
 	// active observers
 	if ctx != nil {
-		// asynchronously wait for the context to unsubscribeAll and unsubscribe
+		// asynchronously wait for the context to be done and then unsubscribe
+		// this observer.
 		go goUnsubscribeOnDone[V](ctx, observer)
 	}
 	return observer
@@ -90,13 +91,12 @@ func (obsvbl *channelObservable[V]) UnsubscribeAll() {
 // unsubscribeAll unsubscribes and removes all observers from the observable.
 func (obsvbl *channelObservable[V]) unsubscribeAll() {
 	// Copy currentObservers to avoid holding the lock while unsubscribing them.
-	// The current observers at this time is the canonical set of observers which
-	// will be unsubscribed.
+	// The observers at the time of locking, prior to copying, are the canonical
+	// set of observers which are unsubscribed.
 	// New or existing Observers may (un)subscribe while the observable is closing.
 	// Any such observers won't be isClosed but will also stop receiving notifications
 	// immediately (if they receive any at all).
 	currentObservers := obsvbl.copyObservers()
-
 	for _, observer := range currentObservers {
 		observer.Unsubscribe()
 	}
@@ -114,12 +114,10 @@ func (obsvbl *channelObservable[V]) goPublish(publisher <-chan V) {
 	for notification := range publisher {
 		// Copy currentObservers to avoid holding the lock while notifying them.
 		// New or existing Observers may (un)subscribe while this notification
-		// is being fanned out to the "current" set  of currentObservers.
-		// The state of currentObservers at this time is the canonical set of currentObservers
-		// which receive this notification.
+		// is being fanned out.
+		// The observers at the time of locking, prior to copying, are the canonical
+		// set of observers which receive this notification.
 		currentObservers := obsvbl.copyObservers()
-
-		// notify currentObservers
 		for _, obsvr := range currentObservers {
 			// TODO_CONSIDERATION: perhaps continue trying to avoid making this
 			// notification async as it would effectively use goroutines
@@ -161,7 +159,7 @@ func goUnsubscribeOnDone[V any](ctx context.Context, subscription observable.Obs
 	subscription.Unsubscribe()
 }
 
-// onUnsubscribe returns a function that removes a given channelObserver from the
+// onUnsubscribe returns a function that removes a given observer from the
 // observable's list of observers.
 func (obsvbl *channelObservable[V]) onUnsubscribe(toRemove *channelObserver[V]) {
 	// must (write) lock to iterate over and modify the observers list
diff --git a/pkg/observable/channel/observable_test.go b/pkg/observable/channel/observable_test.go
index 48f7ba7c4..b3090744f 100644
--- a/pkg/observable/channel/observable_test.go
+++ b/pkg/observable/channel/observable_test.go
@@ -16,9 +16,9 @@ import (
 )
 
 const (
-	publishDelay           = 2 * time.Millisecond
-	notifyTimeout          = publishDelay * 5
-	cancelUnsubscribeDelay = notifyTimeout * 2
+	publishDelay           = 100 * time.Microsecond
+	notifyTimeout          = publishDelay * 20
+	cancelUnsubscribeDelay = publishDelay * 2
 )
 
 func TestChannelObservable_NotifyObservers(t *testing.T) {

From 194c59ad2a580beb4c48c1829232f897854acc56 Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Fri, 20 Oct 2023 08:47:34 +0200
Subject: [PATCH 37/41] docs: add initial docs/pkg/observable docs

---
 docs/pkg/observable/README.md | 191 ++++++++++++++++++++++++++++++++++
 pkg/observable/interface.go   |   2 -
 2 files changed, 191 insertions(+), 2 deletions(-)
 create mode 100644 docs/pkg/observable/README.md

diff --git a/docs/pkg/observable/README.md b/docs/pkg/observable/README.md
new file mode 100644
index 000000000..9523ff457
--- /dev/null
+++ b/docs/pkg/observable/README.md
@@ -0,0 +1,191 @@
+## `pkg/observable` Package
+
+The `pkg/observable` package provides a lightweight and straightforward mechanism to handle asynchronous notifications using the Observer pattern. This is achieved through two primary interfaces: `Observable` and `Observer`.
+
+## Overview
+
+The `Observable` interface is responsible for notifying multiple subscribers about new values asynchronously, while the `Observer` interface allows access to the notified channel and facilitates unsubscribing from an `Observable`.
+
+## Interfaces and Structures
+
+### `Observable` Interface
+
+Represents a publisher in a "Fan-Out" system design, allowing multiple subscribers to be notified of new values asynchronously.
+
+- **Methods**:
+    - **Subscribe**: Used to subscribe an observer to the observable. Returns an instance of the `Observer` interface.
+      ```go
+      func (o *MyObservableType) Subscribe(ctx context.Context) Observer[MyValueType]
+      ```
+    - **UnsubscribeAll**: Unsubscribes all observers from the observable.
+      ```go
+      func (o *MyObservableType) UnsubscribeAll()
+      ```
+
+### `Observer` Interface
+
+Represents a subscriber in a "Fan-Out" system design, providing access to the notified channel and capabilities to unsubscribe.
+
+- **Methods**:
+    - **Unsubscribe**: Used to unsubscribe the observer from its associated observable.
+      ```go
+      func (obs *MyObserverType) Unsubscribe()
+      ```
+    - **Ch**: Returns the channel through which the observer receives notifications.
+      ```go
+      func (obs *MyObserverType) Ch() <-chan MyValueType
+      ```
+
+## Architecture Diagrams
+
+Visual representations often make it easier to understand the design and flow of a package. Below are the architecture diagrams that explain the high-level structure and interactions in this package:
+
+### Observable Synchronization
+
+```mermaid
+flowchart
+    subgraph observable
+        mu1[read/write mutex]
+        sub([#subscribe])
+        close([#close])
+    end
+
+    subgraph observer
+        mu2[read/write mutex]
+        notify([#notify])
+        unsub([#unsubscribe])
+    end
+
+    notify -."read-lock".-> mu2
+sub -."write-lock".-> mu1
+unsub -."write-lock".-> mu1
+unsub -."write-lock".-> mu2
+close -."read-lock".-> mu1
+close -."write-lock".-> mu1
+close --> unsub
+
+observable --> notify
+```
+
+> **Figure 1**: This diagram depicts the synchronization mechanisms between the observable and its observers. It specifically showcases the use of read and write locks for different operations in both observable and observer contexts.
+
+### Observable Buffering
+
+```mermaid
+flowchart
+
+    pub_ctx[publish context]
+    sub_ctx1[subscribe context 1]
+    sub_ctx2[subscribe context 2]
+
+    subgraph observable
+        subgraph pub[publisher channel]
+            pb[publish buffer]
+        end
+    end
+
+    subgraph observer2
+        subgraph sub2[subscriber channel]
+            sb2[subscribe buffer]
+        end
+
+        notify2([#notify])
+    end
+
+    subgraph observer1
+        subgraph sub1[subscriber channel]
+            sb1[subscribe buffer]
+        end
+
+        notify1([#notify])
+    end
+
+    pub_ctx -."source".-> pb
+sb1 -."sink".-> sub_ctx1
+sb2 -."sink".-> sub_ctx2
+
+pb -."sink".-> notify1
+notify1 -."source".-> sb1
+%% pb -."sink".-> sb2
+pb -."sink".-> notify2
+notify2 -."source".-> sb2
+```
+
+> Figure 2: The diagram illustrates the buffering mechanisms within the observable and its observers. It highlights how published messages are buffered and how they propagate to the individual observers' buffers.
+
+## Usage
+
+### Basic Example
+
+```go
+package main
+
+import (
+	"context"
+	"fmt"
+	"time"
+	
+	"poktroll/pkg/observable/channel"
+)
+
+func main() {
+	// Create a new context that can be cancelled
+	ctx, cancel := context.WithCancel(context.Background())
+	// Ensure to cancel the context to release resources
+	defer cancel()
+
+	// Create a new Observable and its corresponding publisher
+	obsvbl, publisher := channel.NewObservable[int]()
+	// Subscribe the first Observer to the Observable
+	observer1 := obsvbl.Subscribe(ctx)
+
+	// Start observing with observer1 in a goroutine
+	go func() {
+		for v := range observer1.Ch() {
+			fmt.Println("Observer1 received:", v)
+		}
+	}()
+
+	// Publish the first value to the Observable
+	publisher <- 10
+	time.Sleep(time.Millisecond)
+
+	// Now, subscribe the second Observer to the Observable
+	observer2 := obsvbl.Subscribe(ctx)
+
+	// Start observing with observer2 in a goroutine
+	go func() {
+		for v := range observer2.Ch() {
+			fmt.Println("Observer2 received:", v)
+		}
+	}()
+
+	// Publish the second value
+	publisher <- 20
+	time.Sleep(time.Millisecond)
+
+	// Unsubscribe observer1 before the last value is sent
+	observer1.Unsubscribe()
+	fmt.Println("Observer1 unsubscribed!")
+
+	// Publish the third value
+	publisher <- 30
+	time.Sleep(time.Millisecond)
+}
+
+// Expected Output:
+// Observer1 received: 10
+// Observer2 received: 20
+// Observer1 received: 20
+// Observer1 unsubscribed!
+// Observer2 received: 30
+
+```
+
+## Considerations
+
+While the `pkg/observable` package is designed to be simple and minimal, developers with more complex requirements may need to consider extending its functionality or exploring other libraries like [RxGo](https://github.com/ReactiveX/RxGo).
+
+## Conclusion
+
+The `pkg/observable` package is an intuitive solution for handling asynchronous notifications in Go projects, ensuring efficient communication between observables and observers.
diff --git a/pkg/observable/interface.go b/pkg/observable/interface.go
index 1e151f4b3..3d4894339 100644
--- a/pkg/observable/interface.go
+++ b/pkg/observable/interface.go
@@ -1,7 +1,5 @@
 package observable
 
-// TODO(@bryanchriswhite): Add a README for the observable.
-
 import "context"
 
 // NOTE: We explicitly decided to write a small and custom notifications package

From fe1b860b853995d6b05f1f3e582bcd3c48076f36 Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Fri, 20 Oct 2023 08:48:00 +0200
Subject: [PATCH 38/41] chore: add go package README.md template

---
 docs/pkg/README_TEMPLATE.md | 98 +++++++++++++++++++++++++++++++++++++
 1 file changed, 98 insertions(+)
 create mode 100644 docs/pkg/README_TEMPLATE.md

diff --git a/docs/pkg/README_TEMPLATE.md b/docs/pkg/README_TEMPLATE.md
new file mode 100644
index 000000000..c7f915e9a
--- /dev/null
+++ b/docs/pkg/README_TEMPLATE.md
@@ -0,0 +1,98 @@
+Certainly! I've added a section named "Architecture Diagrams" in the documentation template below:
+
+```markdown
+# Package [PackageName]
+
+> Brief one-liner or quote about what this package does.
+
+## Overview
+
+Provide a few sentences about the purpose and functionality of this package. Consider:
+- What problems does it solve?
+- Why would someone use this package as opposed to others or implementing their own solution?
+- Any unique features or aspects that stand out.
+
+## Architecture Diagrams
+
+Visual representations often make it easier to understand the design and flow of a package. Below are the architecture diagrams that explain the high-level structure and interactions in this package:
+
+![Architecture Overview](./path-to-diagram1.png)
+
+> **Figure 1**: Brief description about what this diagram represents.
+
+![Another Diagram](./path-to-diagram2.png)
+
+> **Figure 2**: Brief description about what this other diagram represents.
+
+If you have multiple diagrams, you can explain each one separately or provide a list.
+
+## Installation
+
+```bash
+go get github.com/yourusername/yourproject/[PackageName]
+```
+
+## Features
+
+- **Feature 1**: A brief description.
+- **Feature 2**: Another description.
+- ...
+
+## Usage
+
+### Basic Example
+
+```go
+// A simple and concise code example showing the most common use case.
+```
+
+### Advanced Usage
+
+For complex features or functionalities, it's good to have a separate section:
+
+```go
+// Advanced code example or usage.
+```
+
+### Configuration
+
+If the package can be configured in some way, describe it here:
+
+- **Config Option 1**: Explanation.
+- **Config Option 2**: Explanation.
+
+## API Reference
+
+While `godoc` will provide the detailed API reference, you can highlight or briefly describe key functions, types, or methods here.
+
+- `FunctionOrType1()`: A short description of its purpose.
+- `FunctionOrType2(param Type)`: Another brief description.
+
+For the complete API details, see the [godoc](https://pkg.go.dev/github.com/yourusername/yourproject/[PackageName]).
+
+## Best Practices
+
+- **Practice 1**: Description and rationale.
+- **Practice 2**: Another helpful practice.
+
+## FAQ
+
+#### Question 1?
+
+Answer for question 1.
+
+#### Question 2?
+
+Answer for question 2.
+
+## Contributing
+
+Briefly describe how others can contribute to this package. Link to the main contributing guide if you have one.
+
+## Changelog
+
+For detailed release notes, see the [CHANGELOG](../CHANGELOG.md) at the root level or link to a separate CHANGELOG specific to this package.
+
+## License
+
+This package is released under the XYZ License. For more information, see the [LICENSE](../LICENSE) file at the root level.
\ No newline at end of file

From 0d2b0900458ac762ebb52092b415b3a3dbf0c4a6 Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Fri, 20 Oct 2023 08:55:05 +0200
Subject: [PATCH 39/41] chore: fix TODO comments

---
 pkg/observable/channel/observable_test.go | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/pkg/observable/channel/observable_test.go b/pkg/observable/channel/observable_test.go
index b3090744f..224bf38df 100644
--- a/pkg/observable/channel/observable_test.go
+++ b/pkg/observable/channel/observable_test.go
@@ -54,11 +54,12 @@ func TestChannelObservable_NotifyObservers(t *testing.T) {
 			inputs:          inputs,
 			expectedOutputs: inputs,
 		},
-		// INCOMPLETE: publisher channels which are full are proving harder to test
+		// TODO_INCOMPLETE: publisher channels which are full are proving harder to test
 		// robustly (no flakiness); perhaps it has to do with the lack of some
 		// kind of guarantee about the receiver order on the consumer side.
 		//
 		// The following scenarios should generally pass but are flaky:
+		// (see: docs/pkg/observable/README.md regarding synchronization and buffering)
 		//
 		// {
 		// 	name:            "full non-buffered publisher",
@@ -323,7 +324,7 @@ func TestChannelObservable_SequentialPublishAndUnsubscription(t *testing.T) {
 	}
 }
 
-// TECHDEBT/INCOMPLETE: add coverage for active observers closing when publishCh closes.
+// TODO_TECHDEBT/TODO_INCOMPLETE: add coverage for active observers closing when publishCh closes.
 func TestChannelObservable_ObserversCloseOnPublishChannelClose(t *testing.T) {
 	t.Skip("add coverage: all observers should unsubscribeAll when publishCh closes")
 }

From e60fd3582c25325cd6f32c40f6980b5807317f57 Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Fri, 20 Oct 2023 09:00:48 +0200
Subject: [PATCH 40/41] chore: move & rename pkg readme template

---
 docs/{pkg/README_TEMPLATE.md => template/pkg/README.md} | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename docs/{pkg/README_TEMPLATE.md => template/pkg/README.md} (100%)

diff --git a/docs/pkg/README_TEMPLATE.md b/docs/template/pkg/README.md
similarity index 100%
rename from docs/pkg/README_TEMPLATE.md
rename to docs/template/pkg/README.md

From a75576e8d73449e6f5956dbcec2ac2a27424609c Mon Sep 17 00:00:00 2001
From: Bryan White <bryanchriswhite@gmail.com>
Date: Fri, 20 Oct 2023 10:49:09 +0200
Subject: [PATCH 41/41] test: refactor async test errors

---
 internal/testerrors/require.go            | 11 +++++++++++
 pkg/observable/channel/observable_test.go |  5 +++--
 2 files changed, 14 insertions(+), 2 deletions(-)
 create mode 100644 internal/testerrors/require.go

diff --git a/internal/testerrors/require.go b/internal/testerrors/require.go
new file mode 100644
index 000000000..624cdaf6e
--- /dev/null
+++ b/internal/testerrors/require.go
@@ -0,0 +1,11 @@
+package testerrors
+
+import errorsmod "cosmossdk.io/errors"
+
+var (
+	// ErrAsync is returned when a test assertion fails in a goroutine other than
+	// the main test goroutine. This is done to avoid concurrent usage of
+	// t.Fatal() which can cause the test binary to exit before cleanup is complete.
+	ErrAsync  = errorsmod.Register(codespace, 1, "required assertion failed")
+	codespace = "testerrors"
+)
diff --git a/pkg/observable/channel/observable_test.go b/pkg/observable/channel/observable_test.go
index 224bf38df..c6f27929f 100644
--- a/pkg/observable/channel/observable_test.go
+++ b/pkg/observable/channel/observable_test.go
@@ -11,6 +11,7 @@ import (
 	"golang.org/x/sync/errgroup"
 
 	"pocket/internal/testchannel"
+	"pocket/internal/testerrors"
 	"pocket/pkg/observable"
 	"pocket/pkg/observable/channel"
 )
@@ -118,7 +119,7 @@ func TestChannelObservable_NotifyObservers(t *testing.T) {
 						output,
 						"obsvr Idx: %d", obsvrIdx,
 					) {
-						return fmt.Errorf("unexpected output")
+						return testerrors.ErrAsync
 					}
 					return nil
 				}
@@ -130,7 +131,7 @@ func TestChannelObservable_NotifyObservers(t *testing.T) {
 						len(outputs),
 						"obsvr addr: %p", obsvr,
 					) {
-						return fmt.Errorf("unexpected number of outputs")
+						return testerrors.ErrAsync
 					}
 					return nil
 				}