loop/webui: remove end session feedback survey functionality
Remove end session feedback survey system that was previously implemented
to collect user satisfaction ratings and comments when ending sessions.
Problem Analysis:
The feedback survey system added complexity to the end session flow
and included several components:
- Modal dialog with thumbs up/down rating buttons
- Optional text feedback collection
- Backend storage and processing of feedback data
- Complex synchronization logic with skaband clients
- Extended State struct and API surface area
The feedback collection mechanism, while well-implemented, added
unnecessary friction to session termination and required maintenance
of additional state management infrastructure.
Implementation Changes:
1. Frontend Simplification (sketch-app-shell.ts):
- Replace custom survey modal with simple window.confirm dialog
- Remove showEndSessionSurvey() method and associated UI logic
- Simplify end session request to basic reason-only payload
- Remove complex button state management and form handling
2. Type Definition Cleanup (types.ts):
- Remove EndFeedback interface definition
- Remove end field from State interface
- Simplify TypeScript type definitions
3. Backend Refactoring (agent.go):
- Remove EndFeedback struct definition
- Remove GetEndFeedback/SetEndFeedback methods from CodingAgent interface
- Remove endFeedback field from Agent struct
- Eliminate feedback-related state management
4. HTTP Server Cleanup (loophttp.go):
- Remove End field from State struct
- Simplify /end endpoint request body parsing
- Remove feedback storage and processing logic
- Eliminate endWaitGroup synchronization mechanism
- Remove wait_for_end query parameter handling
- Simplify session termination to immediate exit with basic delay
5. Test Cleanup (loophttp_test.go):
- Remove TestEndFeedback test function
- Remove endFeedback field from mockAgent
- Remove GetEndFeedback/SetEndFeedback mock methods
- Simplify test fixtures and expectations
Technical Details:
- Session termination now uses simple confirmation dialog workflow
- /end endpoint accepts only reason field in request body
- Process exit occurs after 100ms delay without client coordination
- All existing session management functionality preserved
- TypeScript compilation verified with simplified type definitions
Benefits:
- Simplified end session user experience with standard confirmation
- Reduced code complexity and maintenance burden
- Eliminated complex state synchronization requirements
- Cleaner API surface with fewer fields and methods
- Faster session termination without feedback collection overhead
- Standard web UI patterns for session ending confirmation
Testing:
- All existing tests pass without regression
- TypeScript compilation succeeds with updated type definitions
- End session functionality verified through manual testing
- Build verification confirms no compilation errors
This change restores the end session flow to a simple, straightforward
confirmation-based approach while maintaining all core session management
functionality and improving overall system simplicity.
Co-Authored-By: sketch <hello@sketch.dev>
Change-ID: s9406002b5ac20a89k
diff --git a/loop/agent.go b/loop/agent.go
index 3326942..f5c6e07 100644
--- a/loop/agent.go
+++ b/loop/agent.go
@@ -36,12 +36,6 @@
userCancelMessage = "user requested agent to stop handling responses"
)
-// EndFeedback represents user feedback when ending a session
-type EndFeedback struct {
- Happy bool `json:"happy"`
- Comment string `json:"comment"`
-}
-
type MessageIterator interface {
// Next blocks until the next message is available. It may
// return nil if the underlying iterator context is done.
@@ -136,10 +130,6 @@
CurrentStateName() string
// CurrentTodoContent returns the current todo list data as JSON, or empty string if no todos exist
CurrentTodoContent() string
- // GetEndFeedback returns the end session feedback
- GetEndFeedback() *EndFeedback
- // SetEndFeedback sets the end session feedback
- SetEndFeedback(feedback *EndFeedback)
// CompactConversation compacts the current conversation by generating a summary
// and restarting the conversation with that summary as the initial context
@@ -398,9 +388,6 @@
// Port monitoring
portMonitor *PortMonitor
-
- // End session feedback
- endFeedback *EndFeedback
}
// NewIterator implements CodingAgent.
@@ -499,20 +486,6 @@
return string(content)
}
-// SetEndFeedback sets the end session feedback
-func (a *Agent) SetEndFeedback(feedback *EndFeedback) {
- a.mu.Lock()
- defer a.mu.Unlock()
- a.endFeedback = feedback
-}
-
-// GetEndFeedback gets the end session feedback
-func (a *Agent) GetEndFeedback() *EndFeedback {
- a.mu.Lock()
- defer a.mu.Unlock()
- return a.endFeedback
-}
-
// generateConversationSummary asks the LLM to create a comprehensive summary of the current conversation
func (a *Agent) generateConversationSummary(ctx context.Context) (string, error) {
msg := `You are being asked to create a comprehensive summary of our conversation so far. This summary will be used to restart our conversation with a shorter history while preserving all important context.
diff --git a/loop/server/loophttp.go b/loop/server/loophttp.go
index 784d20b..0819459 100644
--- a/loop/server/loophttp.go
+++ b/loop/server/loophttp.go
@@ -95,7 +95,6 @@
OutsideWorkingDir string `json:"outside_working_dir,omitempty"`
InsideWorkingDir string `json:"inside_working_dir,omitempty"`
TodoContent string `json:"todo_content,omitempty"` // Contains todo list JSON data
- End *loop.EndFeedback `json:"end,omitempty"` // End session feedback
}
type InitRequest struct {
@@ -734,16 +733,6 @@
endReason = requestBody.Reason
}
- // Store end feedback if provided
- if requestBody.Happy != nil {
- feedback := &loop.EndFeedback{
- Happy: *requestBody.Happy,
- Comment: requestBody.Comment,
- }
- s.agent.SetEndFeedback(feedback)
- slog.Info("End session feedback received", "happy", feedback.Happy, "comment", feedback.Comment)
- }
-
// Send success response before exiting
w.Header().Set("Content-Type", "application/json")
json.NewEncoder(w).Encode(map[string]string{"status": "ending", "reason": endReason})
@@ -1222,12 +1211,6 @@
// Get updated state
state = s.getState()
- // Check if end feedback is present and this client was waiting for it
- if waitForEnd && state.End != nil {
- s.endWaitGroup.Done()
- waitForEnd = false // Mark that we've handled the end condition
- }
-
// Send updated state after the state transition
fmt.Fprintf(w, "event: state\n")
fmt.Fprintf(w, "data: ")
@@ -1255,12 +1238,6 @@
// Get updated state
state = s.getState()
- // Check if end feedback is present and this client was waiting for it
- if waitForEnd && state.End != nil {
- s.endWaitGroup.Done()
- waitForEnd = false // Mark that we've handled the end condition
- }
-
// Send updated state after the message
fmt.Fprintf(w, "event: state\n")
fmt.Fprintf(w, "data: ")
@@ -1307,7 +1284,6 @@
FirstMessageIndex: s.agent.FirstMessageIndex(),
AgentState: s.agent.CurrentStateName(),
TodoContent: s.agent.CurrentTodoContent(),
- End: s.agent.GetEndFeedback(),
}
}
diff --git a/loop/server/loophttp_test.go b/loop/server/loophttp_test.go
index a6dab62..8575253 100644
--- a/loop/server/loophttp_test.go
+++ b/loop/server/loophttp_test.go
@@ -30,7 +30,6 @@
branchName string
workingDir string
sessionID string
- endFeedback *loop.EndFeedback
}
func (m *mockAgent) NewIterator(ctx context.Context, nextMessageIdx int) loop.MessageIterator {
@@ -248,69 +247,8 @@
func (m *mockAgent) IsInContainer() bool { return false }
func (m *mockAgent) FirstMessageIndex() int { return 0 }
func (m *mockAgent) DetectGitChanges(ctx context.Context) error { return nil }
-func (m *mockAgent) GetEndFeedback() *loop.EndFeedback { return m.endFeedback }
-func (m *mockAgent) SetEndFeedback(feedback *loop.EndFeedback) { m.endFeedback = feedback }
-func (m *mockAgent) GetPortMonitor() *loop.PortMonitor { return loop.NewPortMonitor() }
-// TestEndFeedback tests the end session feedback functionality
-func TestEndFeedback(t *testing.T) {
- // Test basic EndFeedback struct functionality
- feedback := &loop.EndFeedback{
- Happy: true,
- Comment: "Great experience!",
- }
-
- if feedback.Happy != true {
- t.Errorf("Expected Happy to be true, got %v", feedback.Happy)
- }
- if feedback.Comment != "Great experience!" {
- t.Errorf("Expected Comment to be 'Great experience!', got %s", feedback.Comment)
- }
-
- // Test mock agent methods
- mockAgent := &mockAgent{
- sessionID: "test-session",
- workingDir: "/test",
- messageCount: 0,
- }
-
- // Test initial state (no feedback)
- if mockAgent.GetEndFeedback() != nil {
- t.Error("Expected initial feedback to be nil")
- }
-
- // Test setting feedback
- mockAgent.SetEndFeedback(feedback)
- retrieved := mockAgent.GetEndFeedback()
- if retrieved == nil {
- t.Error("Expected feedback to be set, got nil")
- } else {
- if retrieved.Happy != true {
- t.Errorf("Expected Happy to be true, got %v", retrieved.Happy)
- }
- if retrieved.Comment != "Great experience!" {
- t.Errorf("Expected Comment to be 'Great experience!', got %s", retrieved.Comment)
- }
- }
-
- // Test setting different feedback
- negativeFeedback := &loop.EndFeedback{
- Happy: false,
- Comment: "Could be better",
- }
- mockAgent.SetEndFeedback(negativeFeedback)
- retrieved = mockAgent.GetEndFeedback()
- if retrieved == nil {
- t.Error("Expected feedback to be set, got nil")
- } else {
- if retrieved.Happy != false {
- t.Errorf("Expected Happy to be false, got %v", retrieved.Happy)
- }
- if retrieved.Comment != "Could be better" {
- t.Errorf("Expected Comment to be 'Could be better', got %s", retrieved.Comment)
- }
- }
-}
+func (m *mockAgent) GetPortMonitor() *loop.PortMonitor { return loop.NewPortMonitor() }
// TestSSEStream tests the SSE stream endpoint
func TestSSEStream(t *testing.T) {