Merge pull request #93 from SaplingLearn/feat/editable-session-names

feat(learn): make tutor session names editable

Author: Jose-Gael-Cruz-Lopez

backend/models/__init__.py

@@ -27,6 +27,11 @@ class EndSessionBody(BaseModel):
     user_id: str = ""  # Required to discard a lazy (not-yet-persisted) session safely
 
 
+class RenameSessionBody(BaseModel):
+    user_id: str
+    topic: str
+
+
 class ActionBody(BaseModel):
     session_id: str
     user_id: str = "user_andres"

backend/routes/learn.py

@@ -14,7 +14,7 @@
 from agents.chat_tutor import agent_for_mode
 from agents.deps import SaplingDeps
 from db.connection import table
-from models import StartSessionBody, ChatBody, EndSessionBody, ActionBody, ModeSwitchBody
+from models import StartSessionBody, ChatBody, EndSessionBody, ActionBody, ModeSwitchBody, RenameSessionBody
 from services.auth_guard import require_self, get_session_user_id
 from services.encryption import encrypt_if_present, encrypt_json, decrypt_if_present, decrypt_json
 from services.gemini_service import (
@@ -713,6 +713,35 @@ def list_sessions(user_id: str, request: Request, limit: int = 10):
     return {"sessions": result}
 
 
+@router.patch("/sessions/{session_id}")
+def rename_session(session_id: str, body: RenameSessionBody, request: Request):
+    require_self(body.user_id, request)
+    topic = body.topic.strip()
+    if not topic or len(topic) > 120:
+        raise HTTPException(status_code=400, detail="Topic must be 1-120 characters")
+
+    if session_id in PENDING_SESSIONS:
+        pending = PENDING_SESSIONS[session_id]
+        if pending["user_id"] != body.user_id:
+            raise HTTPException(status_code=403, detail="Session user mismatch")
+        pending["topic"] = topic
+        return {"updated": True, "session": {"id": session_id, "topic": topic}}
+
+    owner_rows = table("sessions").select(
+        "user_id", filters={"id": f"eq.{session_id}"}, limit=1
+    )
+    if not owner_rows:
+        raise HTTPException(status_code=404, detail="Session not found")
+    if owner_rows[0].get("user_id") != body.user_id:
+        raise HTTPException(status_code=403, detail="Session user mismatch")
+
+    table("sessions").update(
+        {"topic": topic},
+        filters={"id": f"eq.{session_id}"},
+    )
+    return {"updated": True, "session": {"id": session_id, "topic": topic}}
+
+
 @router.delete("/sessions/{session_id}")
 def delete_session(session_id: str, request: Request, user_id: str | None = Query(None)):
     if user_id:

backend/tests/test_learn_routes.py

@@ -281,6 +281,93 @@ def test_message_is_saved_to_db(self):
         assert len(insert_calls) >= 1
 
 
+# ── PATCH /api/learn/sessions/{session_id} ────────────────────────────────────
+
+class TestRenameSession:
+    def test_renames_persisted_session(self):
+        sessions_mock = MagicMock()
+        sessions_mock.select.return_value = [{"user_id": "u1"}]
+
+        def factory(name):
+            if name == "sessions":
+                return sessions_mock
+            m = MagicMock()
+            m.select.return_value = []
+            return m
+
+        with patch("routes.learn.table", side_effect=factory):
+            r = client.patch(
+                "/api/learn/sessions/s1",
+                json={"user_id": "u1", "topic": "  New Topic  "},
+            )
+
+        assert r.status_code == 200
+        body = r.json()
+        assert body == {"updated": True, "session": {"id": "s1", "topic": "New Topic"}}
+        sessions_mock.update.assert_called_once_with(
+            {"topic": "New Topic"},
+            filters={"id": "eq.s1"},
+        )
+
+    def test_renames_pending_session(self):
+        from routes.learn import PENDING_SESSIONS
+        PENDING_SESSIONS["pending-1"] = {
+            "user_id": "u1",
+            "topic": "Old",
+            "mode": "socratic",
+            "assistant_reply": "hi",
+        }
+        try:
+            r = client.patch(
+                "/api/learn/sessions/pending-1",
+                json={"user_id": "u1", "topic": "Renamed"},
+            )
+            assert r.status_code == 200
+            assert r.json() == {"updated": True, "session": {"id": "pending-1", "topic": "Renamed"}}
+            assert PENDING_SESSIONS["pending-1"]["topic"] == "Renamed"
+        finally:
+            PENDING_SESSIONS.pop("pending-1", None)
+
+    def test_empty_topic_returns_400(self):
+        r = client.patch(
+            "/api/learn/sessions/s1",
+            json={"user_id": "u1", "topic": "   "},
+        )
+        assert r.status_code == 400
+
+    def test_topic_too_long_returns_400(self):
+        r = client.patch(
+            "/api/learn/sessions/s1",
+            json={"user_id": "u1", "topic": "x" * 121},
+        )
+        assert r.status_code == 400
+
+    def test_wrong_user_returns_403(self):
+        sessions_mock = MagicMock()
+        sessions_mock.select.return_value = [{"user_id": "other_user"}]
+
+        def factory(name):
+            if name == "sessions":
+                return sessions_mock
+            m = MagicMock()
+            m.select.return_value = []
+            return m
+
+        with patch("routes.learn.table", side_effect=factory):
+            r = client.patch(
+                "/api/learn/sessions/s1",
+                json={"user_id": "u1", "topic": "Renamed"},
+            )
+        assert r.status_code == 403
+
+    def test_missing_session_returns_404(self):
+        with patch("routes.learn.table") as t:
+            t.return_value.select.return_value = []
+            r = client.patch(
+                "/api/learn/sessions/nonexistent",
+                json={"user_id": "u1", "topic": "Renamed"},
+            )
+        assert r.status_code == 404
 # ── _resolve_legacy_model ─────────────────────────────────────────────────────
 
 class TestResolveLegacyModel:

frontend/src/components/screens/Dashboard.tsx

@@ -21,7 +21,7 @@ import {
   type Assignment,
 } from "@/lib/api";
 import type { GraphNode as ApiNode, GraphEdge as ApiEdge } from "@/lib/types";
-import type { GraphNode, GraphEdge } from "@/lib/data";
+import { paletteFor, type GraphNode, type GraphEdge } from "@/lib/data";
 
 const QUOTES = [
   "Learning is the only thing the mind never exhausts, never fears, and never regrets. — da Vinci",
@@ -38,7 +38,10 @@ function apiToGraphNode(n: ApiNode, courses: EnrolledCourse[]): GraphNode {
     id: n.id,
     name: n.concept_name,
     subject: n.subject,
-    color: n.course_color || course?.color || "var(--c-sage)",
+    color:
+      n.course_color ||
+      course?.color ||
+      paletteFor(n.course_id || course?.course_id || n.subject),
     is_subject_root: n.is_subject_root,
     mastery_tier: n.mastery_tier === "subject_root" ? "mastered" : n.mastery_tier,
     mastery_score: n.mastery_score,

frontend/src/components/screens/Learn.tsx

@@ -23,6 +23,7 @@ import {
   getSessions,
   resumeSession,
   deleteSession,
+  renameSession,
   endSession,
   switchMode,
   learnAction,
@@ -33,15 +34,18 @@ import {
   type EnrolledCourse,
 } from "@/lib/api";
 import type { GraphNode as ApiNode, GraphEdge as ApiEdge } from "@/lib/types";
-import type { GraphNode, GraphEdge } from "@/lib/data";
+import { paletteFor, type GraphNode, type GraphEdge } from "@/lib/data";
 
 function apiToGraphNode(n: ApiNode, courses: EnrolledCourse[]): GraphNode {
   const course = courses.find((c) => c.course_name === n.subject);
   return {
     id: n.id,
     name: n.concept_name,
     subject: n.subject,
-    color: n.course_color || course?.color || "var(--c-sage)",
+    color:
+      n.course_color ||
+      course?.color ||
+      paletteFor(n.course_id || course?.course_id || n.subject),
     is_subject_root: n.is_subject_root,
     mastery_tier: n.mastery_tier === "subject_root" ? "mastered" : n.mastery_tier,
     mastery_score: n.mastery_score,
@@ -112,6 +116,8 @@ function LearnInner() {
   const [mobileTab, setMobileTab] = useState<"chat" | "graph">("chat");
   const idCounter = useRef(0);
   const msgId = () => `m-${++idCounter.current}`;
+  // Tracks each session's last server-confirmed topic so back-to-back rename failures revert to the right value.
+  const confirmedTopicsRef = useRef<Map<string, string>>(new Map());
 
   // Initial data load
   useEffect(() => {
@@ -125,7 +131,11 @@ function LearnInner() {
           getGraph(userId).catch(() => ({ nodes: [] as any[], edges: [] as any[], stats: {} })),
         ]);
         if (cancelled) return;
-        setRecentSessions((sRes.sessions ?? []).filter(s => s.message_count > 0));
+        const filteredSessions = (sRes.sessions ?? []).filter(s => s.message_count > 0);
+        setRecentSessions(filteredSessions);
+        confirmedTopicsRef.current = new Map(
+          filteredSessions.map(s => [s.id, s.topic] as const),
+        );
         setCourses(cRes.courses ?? []);
         const nodes = (gRes.nodes ?? []) as Array<{ id: string; concept_name?: string; name?: string; course_id?: string | null; is_subject_root?: boolean }>;
         const courseById = new Map((cRes.courses ?? []).map(c => [c.course_id, c]));
@@ -208,6 +218,27 @@ function LearnInner() {
     }
   };
 
+  const handleRenameSession = useCallback(async (s: Session, newTopic: string) => {
+    if (!userId) return;
+    const trimmed = newTopic.trim();
+    if (!trimmed || trimmed.length > 120 || trimmed === s.topic) return;
+    setRecentSessions(prev => prev.map(p => (p.id === s.id ? { ...p, topic: trimmed } : p)));
+    try {
+      await renameSession(s.id, userId, trimmed);
+      confirmedTopicsRef.current.set(s.id, trimmed);
+    } catch (err) {
+      // Server is authoritative — resync from it instead of guessing a revert
+      // target, which is otherwise racy when multiple renames overlap.
+      const res = await getSessions(userId, 10).catch(() => null);
+      if (res) {
+        const filtered = (res.sessions ?? []).filter(x => x.message_count > 0);
+        setRecentSessions(filtered);
+        confirmedTopicsRef.current = new Map(filtered.map(x => [x.id, x.topic] as const));
+      }
+      toast.error(err instanceof Error ? err.message : "Rename failed.");
+    }
+  }, [userId, toast]);
+
   const send = useCallback(async (userText: string) => {
     if (!userText.trim() || !sessionId || !userId) return;
     setMessages(m => [
@@ -488,7 +519,7 @@ function LearnInner() {
               <div style={{ fontSize: 12, color: "var(--text-muted)" }}>No recent sessions yet.</div>
             )}
             {recentSessions.map(s => (
-              <SessionRow key={s.id} s={s} onResume={handleResume} onDelete={handleDeleteSession} />
+              <SessionRow key={s.id} s={s} onResume={handleResume} onDelete={handleDeleteSession} onRename={handleRenameSession} />
             ))}
           </div>
         </div>
@@ -712,12 +743,41 @@ function BackToLearnLink({ onClick }: { onClick: () => void }) {
   );
 }
 
-function SessionRow({ s, onResume, onDelete }: {
+function SessionRow({ s, onResume, onDelete, onRename }: {
   s: Session;
   onResume: (s: Session) => void;
   onDelete: (s: Session) => void;
+  onRename: (s: Session, newTopic: string) => void;
 }) {
   const del = useConfirm(() => onDelete(s), 3000);
+  const [editing, setEditing] = useState(false);
+  const [draft, setDraft] = useState(s.topic);
+  // Esc unmounts the input, which fires blur → commitEdit. The blur closure
+  // still holds the typed `draft`, so without this guard Esc would commit.
+  const cancellingRef = useRef(false);
+
+  const startEdit = () => {
+    setDraft(s.topic);
+    setEditing(true);
+  };
+
+  const commitEdit = () => {
+    if (cancellingRef.current) {
+      cancellingRef.current = false;
+      setEditing(false);
+      return;
+    }
+    const trimmed = draft.trim();
+    if (trimmed && trimmed !== s.topic) onRename(s, trimmed);
+    setEditing(false);
+  };
+
+  const cancelEdit = () => {
+    cancellingRef.current = true;
+    setDraft(s.topic);
+    setEditing(false);
+  };
+
   return (
     <div
       style={{
@@ -730,24 +790,70 @@ function SessionRow({ s, onResume, onDelete }: {
         marginBottom: 6,
       }}
     >
-      <button
-        onClick={() => onResume(s)}
-        style={{
-          flex: 1,
-          textAlign: "left",
-          display: "flex",
-          flexDirection: "column",
-          gap: 2,
-        }}
-      >
-        <span style={{ fontSize: 13, fontWeight: 600 }}>{s.topic}</span>
-        <span style={{ fontSize: 11, color: "var(--text-muted)" }}>
-          {s.mode} · {s.message_count} msg{s.message_count === 1 ? "" : "s"}
-        </span>
-      </button>
+      {editing ? (
+        <div style={{ flex: 1, display: "flex", flexDirection: "column", gap: 2 }}>
+          <input
+            autoFocus
+            aria-label="Session name"
+            value={draft}
+            maxLength={120}
+            onChange={e => setDraft(e.target.value)}
+            onKeyDown={e => {
+              if (e.key === "Enter") {
+                e.preventDefault();
+                commitEdit();
+              } else if (e.key === "Escape") {
+                e.preventDefault();
+                cancelEdit();
+              }
+            }}
+            onBlur={commitEdit}
+            style={{
+              fontSize: 13,
+              fontWeight: 600,
+              padding: "2px 4px",
+              border: "1px solid var(--border)",
+              borderRadius: "var(--r-sm)",
+              background: "var(--bg)",
+              color: "var(--text)",
+              outline: "none",
+            }}
+          />
+          <span style={{ fontSize: 11, color: "var(--text-muted)" }}>
+            {s.mode} · {s.message_count} msg{s.message_count === 1 ? "" : "s"}
+          </span>
+        </div>
+      ) : (
+        <button
+          onClick={() => onResume(s)}
+          style={{
+            flex: 1,
+            textAlign: "left",
+            display: "flex",
+            flexDirection: "column",
+            gap: 2,
+          }}
+        >
+          <span style={{ fontSize: 13, fontWeight: 600 }}>{s.topic}</span>
+          <span style={{ fontSize: 11, color: "var(--text-muted)" }}>
+            {s.mode} · {s.message_count} msg{s.message_count === 1 ? "" : "s"}
+          </span>
+        </button>
+      )}
+      {!editing && (
+        <button
+          className="btn btn--ghost btn--sm"
+          onClick={startEdit}
+          aria-label="Rename session"
+          title="Rename"
+        >
+          <Icon name="pencil" size={12} />
+        </button>
+      )}
       <button
         className={del.armed ? "btn btn--danger btn--sm" : "btn btn--ghost btn--sm"}
         onClick={del.trigger}
+        disabled={editing}
         aria-label={del.armed ? "Confirm delete" : "Delete session"}
         title={del.armed ? "Click again to confirm" : "Delete"}
       >