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) {
diff --git a/webui/src/types.ts b/webui/src/types.ts
index 5bb9f55..53cc96f 100644
--- a/webui/src/types.ts
+++ b/webui/src/types.ts
@@ -61,11 +61,6 @@
tool_uses: { [key: string]: number } | null;
}
-export interface EndFeedback {
- happy: boolean;
- comment: string;
-}
-
export interface State {
state_version: number;
message_count: number;
@@ -92,7 +87,6 @@
outside_working_dir?: string;
inside_working_dir?: string;
todo_content?: string;
- end?: EndFeedback | null;
}
export interface TodoItem {
diff --git a/webui/src/web-components/sketch-app-shell.ts b/webui/src/web-components/sketch-app-shell.ts
index 59fb921..07c355d 100644
--- a/webui/src/web-components/sketch-app-shell.ts
+++ b/webui/src/web-components/sketch-app-shell.ts
@@ -1015,25 +1015,19 @@
event.stopPropagation();
}
- // Show custom dialog with survey
- const surveyResult = await this.showEndSessionSurvey();
- if (!surveyResult) return; // User cancelled
+ // Show confirmation dialog
+ const confirmed = window.confirm(
+ "Ending the session will shut down the underlying container. Are you sure?",
+ );
+ if (!confirmed) return;
try {
- const requestBody: any = { reason: "user requested end of session" };
-
- // Add survey data if provided
- if (surveyResult.happy !== null) {
- requestBody.happy = surveyResult.happy;
- requestBody.comment = surveyResult.comment;
- }
-
const response = await fetch("end", {
method: "POST",
headers: {
"Content-Type": "application/json",
},
- body: JSON.stringify(requestBody),
+ body: JSON.stringify({ reason: "user requested end of session" }),
});
if (!response.ok) {
@@ -1162,173 +1156,6 @@
this.style.setProperty("--chat-input-height", `${bottomOffset}px`);
}
- /**
- * Show end session survey dialog
- */
- private async showEndSessionSurvey(): Promise<{
- happy: boolean | null;
- comment: string;
- } | null> {
- return new Promise((resolve) => {
- // Create modal overlay
- const overlay = document.createElement("div");
- overlay.style.cssText = `
- position: fixed;
- top: 0;
- left: 0;
- right: 0;
- bottom: 0;
- background: rgba(0, 0, 0, 0.5);
- display: flex;
- align-items: center;
- justify-content: center;
- z-index: 10000;
- `;
-
- // Create modal content
- const modal = document.createElement("div");
- modal.style.cssText = `
- background: white;
- border-radius: 8px;
- padding: 24px;
- max-width: 500px;
- width: 90%;
- box-shadow: 0 4px 12px rgba(0, 0, 0, 0.15);
- `;
-
- modal.innerHTML = `
- <h3 style="margin: 0 0 16px 0; font-size: 18px; font-weight: 600;">End Session</h3>
- <p style="margin: 0 0 20px 0; color: #666;">Ending the session will shut down the underlying container. Are you sure?</p>
-
- <div style="margin-bottom: 20px;">
- <p style="margin: 0 0 12px 0; font-weight: 500;">How was your experience?</p>
- <div style="display: flex; gap: 12px; margin-bottom: 16px;">
- <button id="thumbs-up" style="
- background: #f8f9fa;
- border: 2px solid #dee2e6;
- border-radius: 6px;
- padding: 8px 16px;
- cursor: pointer;
- display: flex;
- align-items: center;
- gap: 8px;
- font-size: 14px;
- ">
- 👍 Good
- </button>
- <button id="thumbs-down" style="
- background: #f8f9fa;
- border: 2px solid #dee2e6;
- border-radius: 6px;
- padding: 8px 16px;
- cursor: pointer;
- display: flex;
- align-items: center;
- gap: 8px;
- font-size: 14px;
- ">
- 👎 Not so good
- </button>
- </div>
-
- <label style="display: block; margin-bottom: 8px; font-weight: 500;">Any additional feedback? (optional)</label>
- <textarea id="feedback-text" placeholder="Tell us what went well or what could be improved..." style="
- width: 100%;
- min-height: 80px;
- padding: 8px;
- border: 1px solid #ccc;
- border-radius: 4px;
- resize: vertical;
- font-family: inherit;
- font-size: 14px;
- box-sizing: border-box;
- "></textarea>
- </div>
-
- <div style="display: flex; gap: 12px; justify-content: flex-end;">
- <button id="cancel-btn" style="
- background: #f8f9fa;
- border: 1px solid #dee2e6;
- color: #495057;
- padding: 8px 16px;
- border-radius: 4px;
- cursor: pointer;
- ">Cancel</button>
- <button id="end-btn" style="
- background: #dc3545;
- border: 1px solid #dc3545;
- color: white;
- padding: 8px 16px;
- border-radius: 4px;
- cursor: pointer;
- ">End Session</button>
- </div>
- `;
-
- overlay.appendChild(modal);
- document.body.appendChild(overlay);
-
- let selectedRating: boolean | null = null;
-
- // Handle thumbs up/down selection
- const thumbsUp = modal.querySelector("#thumbs-up") as HTMLButtonElement;
- const thumbsDown = modal.querySelector(
- "#thumbs-down",
- ) as HTMLButtonElement;
- const feedbackText = modal.querySelector(
- "#feedback-text",
- ) as HTMLTextAreaElement;
- const cancelBtn = modal.querySelector("#cancel-btn") as HTMLButtonElement;
- const endBtn = modal.querySelector("#end-btn") as HTMLButtonElement;
-
- const updateButtonStyles = () => {
- thumbsUp.style.background =
- selectedRating === true ? "#d4edda" : "#f8f9fa";
- thumbsUp.style.borderColor =
- selectedRating === true ? "#28a745" : "#dee2e6";
- thumbsDown.style.background =
- selectedRating === false ? "#f8d7da" : "#f8f9fa";
- thumbsDown.style.borderColor =
- selectedRating === false ? "#dc3545" : "#dee2e6";
- };
-
- thumbsUp.addEventListener("click", () => {
- selectedRating = true;
- updateButtonStyles();
- });
-
- thumbsDown.addEventListener("click", () => {
- selectedRating = false;
- updateButtonStyles();
- });
-
- cancelBtn.addEventListener("click", () => {
- document.body.removeChild(overlay);
- resolve(null);
- });
-
- endBtn.addEventListener("click", () => {
- const result = {
- happy: selectedRating,
- comment: feedbackText.value.trim(),
- };
- document.body.removeChild(overlay);
- resolve(result);
- });
-
- // Close on overlay click
- overlay.addEventListener("click", (e) => {
- if (e.target === overlay) {
- document.body.removeChild(overlay);
- resolve(null);
- }
- });
-
- // Focus the modal
- modal.focus();
- });
- }
-
render() {
return html`
<div id="top-banner">