diff --git a/server/internal/domain/types.go b/server/internal/domain/types.go index 1cb48f4b..e73d0408 100644 --- a/server/internal/domain/types.go +++ b/server/internal/domain/types.go @@ -286,23 +286,37 @@ type Session struct { // bumps Version. The sessions table is never modified, so the overlay only // changes how Session Search renders an already-matched row. type SessionEdit struct { - ID string `json:"id"` // == sessions.id - AppID string `json:"appId,omitempty"` - SessionID string `json:"session_id,omitempty"` - Seq int `json:"seq"` - AgentID string `json:"agent_id,omitempty"` - OriginalContent string `json:"original_content"` - EditedContent string `json:"edited_content"` - EditedTags []string `json:"edited_tags,omitempty"` + ID string `json:"id"` // == sessions.id + AppID string `json:"appId,omitempty"` + SessionID string `json:"session_id,omitempty"` + Seq int `json:"seq"` + AgentID string `json:"agent_id,omitempty"` + OriginalContent string `json:"original_content"` + EditedContent string `json:"edited_content,omitempty"` + // EditedContentSet mirrors whether the edited_content column is non-NULL. + // A row can exist with only a correctness mark and no content override + // (mark-only row); the overlay replaces rendered content only when this + // is true. + EditedContentSet bool `json:"-"` + EditedTags []string `json:"edited_tags,omitempty"` // EditedTagsSet distinguishes "tags omitted (leave display tags as-is)" // from "tags explicitly set (including [] to clear)". It mirrors whether // the edited_tags column is non-NULL; only when true does the overlay // replace a row's rendered tags. - EditedTagsSet bool `json:"-"` - EditedBy string `json:"edited_by,omitempty"` - Reason string `json:"reason,omitempty"` - Version int `json:"version"` - State MemoryState `json:"state"` - CreatedAt time.Time `json:"created_at"` - UpdatedAt time.Time `json:"updated_at"` + EditedTagsSet bool `json:"-"` + // Correctness is a human review mark: "correct" or "incorrect"; empty + // means unmarked. A content edit auto-sets it to "correct". + Correctness string `json:"correctness,omitempty"` + EditedBy string `json:"edited_by,omitempty"` + Reason string `json:"reason,omitempty"` + Version int `json:"version"` + State MemoryState `json:"state"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` } + +// SessionEdit correctness mark values. +const ( + SessionEditCorrect = "correct" + SessionEditIncorrect = "incorrect" +) diff --git a/server/internal/handler/handler.go b/server/internal/handler/handler.go index 26eaaaa7..a6dc9713 100644 --- a/server/internal/handler/handler.go +++ b/server/internal/handler/handler.go @@ -291,6 +291,9 @@ func (s *Server) Router( r.Put("/session-messages/{id}", s.editSessionMessage) r.Get("/session-messages/{id}/edit", s.getSessionMessageEdit) r.Delete("/session-messages/{id}/edit", s.deleteSessionMessageEdit) + // Correctness review mark (correct|incorrect) on a raw session turn. + r.Put("/session-messages/{id}/mark", s.markSessionMessage) + r.Delete("/session-messages/{id}/mark", s.unmarkSessionMessage) }) return r diff --git a/server/internal/handler/memory.go b/server/internal/handler/memory.go index 9876092c..24360eca 100644 --- a/server/internal/handler/memory.go +++ b/server/internal/handler/memory.go @@ -1813,6 +1813,10 @@ type sessionMessageResponse struct { CreatedAt time.Time `json:"created_at"` UpdatedAt time.Time `json:"updated_at"` ChainSource *domain.ChainSource `json:"chain_source,omitempty"` + // Correctness review mark (correct|incorrect) if the turn has been + // marked. Raw browse shows the mark but never the content overlay — the + // content here is always the original session text. + Correctness string `json:"correctness,omitempty"` } func (s *Server) handleListSessionMessages(w http.ResponseWriter, r *http.Request) { @@ -1875,6 +1879,13 @@ func (s *Server) handleListSessionMessages(w http.ResponseWriter, r *http.Reques sessions = []*domain.Session{} } messages := make([]sessionMessageResponse, len(sessions)) + ids := make([]string, 0, len(sessions)) + for _, sess := range sessions { + ids = append(ids, sess.ID) + } + // Annotate raw turns with their correctness mark (if any). Content stays + // original — raw browse never applies the content overlay. + marks := svc.session.SessionCorrectnessByIDs(r.Context(), ids) for i, sess := range sessions { messages[i] = sessionMessageResponse{ ID: sess.ID, @@ -1890,6 +1901,7 @@ func (s *Server) handleListSessionMessages(w http.ResponseWriter, r *http.Reques State: sess.State, CreatedAt: sess.CreatedAt, UpdatedAt: sess.UpdatedAt, + Correctness: marks[sess.ID], } } respond(w, http.StatusOK, map[string]any{ diff --git a/server/internal/handler/memory_test.go b/server/internal/handler/memory_test.go index cbfaf0da..5b2e6fa8 100644 --- a/server/internal/handler/memory_test.go +++ b/server/internal/handler/memory_test.go @@ -362,6 +362,44 @@ func (s *testSessionRepo) DeleteSessionEdit(_ context.Context, id string) (int64 return 0, nil } +func (s *testSessionRepo) MarkSessionEdit(_ context.Context, edit *domain.SessionEdit) error { + s.mu.Lock() + defer s.mu.Unlock() + if s.overlays == nil { + s.overlays = map[string]*domain.SessionEdit{} + } + if existing, ok := s.overlays[edit.ID]; ok { + existing.Correctness = edit.Correctness // preserve content/tags + existing.Version++ + return nil + } + cp := *edit // mark-only: no content override + cp.EditedContent = "" + cp.EditedContentSet = false + cp.Version = 1 + if cp.State == "" { + cp.State = domain.StateActive + } + s.overlays[edit.ID] = &cp + return nil +} + +func (s *testSessionRepo) ClearSessionEditMark(_ context.Context, id string) (bool, error) { + s.mu.Lock() + defer s.mu.Unlock() + ov, ok := s.overlays[id] + if !ok || ov.Correctness == "" { + return false, nil + } + if !ov.EditedContentSet { + delete(s.overlays, id) // mark-only row → remove + return true, nil + } + ov.Correctness = "" + ov.Version++ + return true, nil +} + func intPtr(v int) *int { return &v } diff --git a/server/internal/handler/session_edit.go b/server/internal/handler/session_edit.go index 5938ea4e..357a948b 100644 --- a/server/internal/handler/session_edit.go +++ b/server/internal/handler/session_edit.go @@ -112,3 +112,61 @@ func (s *Server) deleteSessionMessageEdit(w http.ResponseWriter, r *http.Request } respond(w, http.StatusOK, map[string]any{"id": id, "reverted": removed > 0}) } + +type markSessionMessageRequest struct { + Correctness string `json:"correctness"` +} + +// markSessionMessage handles PUT /session-messages/{id}/mark: set the +// correctness review mark (correct|incorrect) on a raw session row without +// touching any content/tag overlay. +func (s *Server) markSessionMessage(w http.ResponseWriter, r *http.Request) { + auth := authInfo(r) + if auth.IsChain() { + s.handleError(r.Context(), w, &domain.ValidationError{ + Field: "session_edit", Message: "raw session edit is not supported on chain keys", + }) + return + } + id := chi.URLParam(r, "id") + if strings.TrimSpace(id) == "" { + s.handleError(r.Context(), w, &domain.ValidationError{Field: "id", Message: "required"}) + return + } + var req markSessionMessageRequest + if err := decode(r, &req); err != nil { + s.handleError(r.Context(), w, err) + return + } + svc := s.resolveServices(auth) + edit, err := svc.session.MarkSessionOverlay(r.Context(), id, strings.TrimSpace(req.Correctness), auth.AgentName) + if err != nil { + s.handleError(r.Context(), w, err) + return + } + respond(w, http.StatusOK, map[string]any{ + "id": edit.ID, + "correctness": edit.Correctness, + "version": edit.Version, + }) +} + +// unmarkSessionMessage handles DELETE /session-messages/{id}/mark: clear the +// correctness mark (and remove the overlay row if it had no content edit). +func (s *Server) unmarkSessionMessage(w http.ResponseWriter, r *http.Request) { + auth := authInfo(r) + if auth.IsChain() { + s.handleError(r.Context(), w, &domain.ValidationError{ + Field: "session_edit", Message: "raw session edit is not supported on chain keys", + }) + return + } + id := chi.URLParam(r, "id") + svc := s.resolveServices(auth) + cleared, err := svc.session.ClearSessionOverlayMark(r.Context(), id) + if err != nil { + s.handleError(r.Context(), w, err) + return + } + respond(w, http.StatusOK, map[string]any{"id": id, "unmarked": cleared}) +} diff --git a/server/internal/handler/session_edit_test.go b/server/internal/handler/session_edit_test.go index 5fb64ce1..ded3a1c9 100644 --- a/server/internal/handler/session_edit_test.go +++ b/server/internal/handler/session_edit_test.go @@ -215,3 +215,166 @@ func TestSessionSearch_ExplicitTagsOverride(t *testing.T) { t.Fatalf("explicit tags must override, got %v", resp.Memories[0].Tags) } } + +// --- correctness mark --- + +func TestMarkSessionMessage_SetsAndUpdates(t *testing.T) { + sessionRepo := &testSessionRepo{getResult: sessionRow("turn-1", "original text")} + srv := newTestServer(&testMemoryRepo{}, sessionRepo) + + mark := func(status string) map[string]any { + req := withURLParam(makeRequest(t, http.MethodPut, "/session-messages/turn-1/mark", + markSessionMessageRequest{Correctness: status}), "id", "turn-1") + rr := httptest.NewRecorder() + srv.markSessionMessage(rr, req) + if rr.Code != http.StatusOK { + t.Fatalf("mark %s status = %d: %s", status, rr.Code, rr.Body.String()) + } + var resp map[string]any + _ = json.NewDecoder(rr.Body).Decode(&resp) + return resp + } + + r1 := mark("correct") + if r1["correctness"] != "correct" || r1["version"].(float64) != 1 { + t.Fatalf("first mark = %v, want correct/v1", r1) + } + r2 := mark("incorrect") + if r2["correctness"] != "incorrect" || r2["version"].(float64) != 2 { + t.Fatalf("re-mark = %v, want incorrect/v2", r2) + } + // Mark-only: no content override stored. + if ov := sessionRepo.overlays["turn-1"]; ov == nil || ov.EditedContentSet { + t.Fatalf("mark-only row must have no content override: %+v", ov) + } +} + +func TestMarkSessionMessage_InvalidValue(t *testing.T) { + srv := newTestServer(&testMemoryRepo{}, &testSessionRepo{getResult: sessionRow("turn-1", "x")}) + req := withURLParam(makeRequest(t, http.MethodPut, "/session-messages/turn-1/mark", + markSessionMessageRequest{Correctness: "bogus"}), "id", "turn-1") + rr := httptest.NewRecorder() + srv.markSessionMessage(rr, req) + if rr.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400", rr.Code) + } +} + +func TestMarkSessionMessage_NotFound(t *testing.T) { + srv := newTestServer(&testMemoryRepo{}, &testSessionRepo{}) + req := withURLParam(makeRequest(t, http.MethodPut, "/session-messages/missing/mark", + markSessionMessageRequest{Correctness: "correct"}), "id", "missing") + rr := httptest.NewRecorder() + srv.markSessionMessage(rr, req) + if rr.Code != http.StatusNotFound { + t.Fatalf("status = %d, want 404", rr.Code) + } +} + +func TestSessionSearch_MarkOnlySurfacesCorrectnessNoContentChange(t *testing.T) { + row := sessionRow("turn-1", "original text") + sessionRepo := &testSessionRepo{getResult: row, listResults: []domain.Memory{*row}, listTotal: 1} + srv := newTestServer(&testMemoryRepo{}, sessionRepo) + + srv.markSessionMessage(httptest.NewRecorder(), withURLParam(makeRequest(t, http.MethodPut, + "/session-messages/turn-1/mark", markSessionMessageRequest{Correctness: "incorrect"}), "id", "turn-1")) + + rr := httptest.NewRecorder() + srv.listMemories(rr, makeRequest(t, http.MethodGet, "/memories?memory_type=session", nil)) + var resp listResponse + _ = json.NewDecoder(rr.Body).Decode(&resp) + if len(resp.Memories) != 1 || resp.Memories[0].Content != "original text" { + t.Fatalf("mark-only must not change content: %+v", resp.Memories) + } + var meta map[string]any + _ = json.Unmarshal(resp.Memories[0].Metadata, &meta) + if meta["correctness"] != "incorrect" { + t.Fatalf("correctness not surfaced: %v", meta) + } + if _, edited := meta["edited"]; edited { + t.Fatalf("mark-only must not set edited marker: %v", meta) + } +} + +func TestEditSessionMessage_AutoSetsCorrect(t *testing.T) { + sessionRepo := &testSessionRepo{getResult: sessionRow("turn-1", "original text")} + srv := newTestServer(&testMemoryRepo{}, sessionRepo) + + srv.editSessionMessage(httptest.NewRecorder(), withURLParam(makeRequest(t, http.MethodPut, + "/session-messages/turn-1", editSessionMessageRequest{Content: "fixed text"}), "id", "turn-1")) + + rr := httptest.NewRecorder() + srv.getSessionMessageEdit(rr, withURLParam(makeRequest(t, http.MethodGet, + "/session-messages/turn-1/edit", nil), "id", "turn-1")) + var ov domain.SessionEdit + _ = json.NewDecoder(rr.Body).Decode(&ov) + if ov.Correctness != "correct" { + t.Fatalf("content edit must auto-set correct, got %q", ov.Correctness) + } +} + +func TestUnmarkSessionMessage_DeletesMarkOnlyRow(t *testing.T) { + sessionRepo := &testSessionRepo{getResult: sessionRow("turn-1", "original text")} + srv := newTestServer(&testMemoryRepo{}, sessionRepo) + + srv.markSessionMessage(httptest.NewRecorder(), withURLParam(makeRequest(t, http.MethodPut, + "/session-messages/turn-1/mark", markSessionMessageRequest{Correctness: "incorrect"}), "id", "turn-1")) + dr := httptest.NewRecorder() + srv.unmarkSessionMessage(dr, withURLParam(makeRequest(t, http.MethodDelete, + "/session-messages/turn-1/mark", nil), "id", "turn-1")) + if dr.Code != http.StatusOK { + t.Fatalf("unmark status = %d", dr.Code) + } + if _, ok := sessionRepo.overlays["turn-1"]; ok { + t.Fatal("mark-only row must be removed after unmark") + } +} + +func TestUnmarkSessionMessage_PreservesContentEdit(t *testing.T) { + sessionRepo := &testSessionRepo{getResult: sessionRow("turn-1", "original text")} + srv := newTestServer(&testMemoryRepo{}, sessionRepo) + + // Content edit (auto-correct), then unmark — content overlay must survive. + srv.editSessionMessage(httptest.NewRecorder(), withURLParam(makeRequest(t, http.MethodPut, + "/session-messages/turn-1", editSessionMessageRequest{Content: "fixed text"}), "id", "turn-1")) + srv.unmarkSessionMessage(httptest.NewRecorder(), withURLParam(makeRequest(t, http.MethodDelete, + "/session-messages/turn-1/mark", nil), "id", "turn-1")) + + ov := sessionRepo.overlays["turn-1"] + if ov == nil || ov.EditedContent != "fixed text" || ov.Correctness != "" { + t.Fatalf("unmark must keep content edit and clear mark: %+v", ov) + } +} + +func TestRawBrowse_SurfacesCorrectnessNotContent(t *testing.T) { + row := sessionRow("turn-1", "original text") + sessionRepo := &testSessionRepo{ + getResult: row, + sessionListResults: []*domain.Session{{ID: "turn-1", SessionID: "sess-1", Seq: 3, Role: "user", Content: "original text", ContentType: "text", State: domain.StateActive}}, + } + srv := newTestServer(&testMemoryRepo{}, sessionRepo) + + // Edit content (overlay) AND it auto-marks correct. + srv.editSessionMessage(httptest.NewRecorder(), withURLParam(makeRequest(t, http.MethodPut, + "/session-messages/turn-1", editSessionMessageRequest{Content: "edited text"}), "id", "turn-1")) + + rr := httptest.NewRecorder() + srv.handleListSessionMessages(rr, makeRequest(t, http.MethodGet, "/session-messages?session_id=sess-1", nil)) + if rr.Code != http.StatusOK { + t.Fatalf("status = %d: %s", rr.Code, rr.Body.String()) + } + var resp struct { + Messages []sessionMessageResponse `json:"messages"` + } + _ = json.NewDecoder(rr.Body).Decode(&resp) + if len(resp.Messages) != 1 { + t.Fatalf("want 1 message, got %d", len(resp.Messages)) + } + // Raw browse: original content, but correctness annotation present. + if resp.Messages[0].Content != "original text" { + t.Fatalf("raw browse must show original content, got %q", resp.Messages[0].Content) + } + if resp.Messages[0].Correctness != "correct" { + t.Fatalf("raw browse must surface correctness, got %q", resp.Messages[0].Correctness) + } +} diff --git a/server/internal/repository/factory.go b/server/internal/repository/factory.go index c0c9bd20..c40c3057 100644 --- a/server/internal/repository/factory.go +++ b/server/internal/repository/factory.go @@ -151,3 +151,9 @@ func (stubSessionRepo) GetSessionEditsByIDs(_ context.Context, _ []string) (map[ func (stubSessionRepo) DeleteSessionEdit(_ context.Context, _ string) (int64, error) { return 0, fmt.Errorf("session edit: %w", domain.ErrNotSupported) } +func (stubSessionRepo) MarkSessionEdit(_ context.Context, _ *domain.SessionEdit) error { + return fmt.Errorf("session edit: %w", domain.ErrNotSupported) +} +func (stubSessionRepo) ClearSessionEditMark(_ context.Context, _ string) (bool, error) { + return false, fmt.Errorf("session edit: %w", domain.ErrNotSupported) +} diff --git a/server/internal/repository/repository.go b/server/internal/repository/repository.go index ddeb380d..2da2dfbb 100644 --- a/server/internal/repository/repository.go +++ b/server/internal/repository/repository.go @@ -131,4 +131,9 @@ type SessionRepo interface { GetSessionEdit(ctx context.Context, id string) (*domain.SessionEdit, error) GetSessionEditsByIDs(ctx context.Context, ids []string) (map[string]*domain.SessionEdit, error) DeleteSessionEdit(ctx context.Context, id string) (int64, error) + // MarkSessionEdit upserts only the correctness mark (correct|incorrect), + // preserving any existing content/tag overlay. ClearSessionEditMark + // removes the mark and deletes the row if it had no content override. + MarkSessionEdit(ctx context.Context, edit *domain.SessionEdit) error + ClearSessionEditMark(ctx context.Context, id string) (bool, error) } diff --git a/server/internal/repository/tidb/session_edits.go b/server/internal/repository/tidb/session_edits.go index b4d91ed5..b054905d 100644 --- a/server/internal/repository/tidb/session_edits.go +++ b/server/internal/repository/tidb/session_edits.go @@ -37,18 +37,19 @@ func (r *SessionRepo) UpsertSessionEdit(ctx context.Context, edit *domain.Sessio } _, err := r.db.ExecContext(ctx, `INSERT INTO session_edits - (id, app_id, session_id, seq, agent_id, original_content, edited_content, edited_tags, edited_by, reason, version, state, created_at, updated_at) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, 1, ?, NOW(), NOW()) + (id, app_id, session_id, seq, agent_id, original_content, edited_content, edited_tags, correctness, edited_by, reason, version, state, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, 1, ?, NOW(), NOW()) ON DUPLICATE KEY UPDATE edited_content = VALUES(edited_content), edited_tags = COALESCE(VALUES(edited_tags), edited_tags), + correctness = VALUES(correctness), edited_by = VALUES(edited_by), reason = VALUES(reason), version = version + 1, state = VALUES(state), updated_at = NOW()`, edit.ID, edit.AppID, nullStr(edit.SessionID), edit.Seq, nullStr(edit.AgentID), - edit.OriginalContent, edit.EditedContent, editedTags, + edit.OriginalContent, edit.EditedContent, editedTags, nullStr(edit.Correctness), nullStr(edit.EditedBy), nullStr(edit.Reason), string(state), ) if err != nil { @@ -57,10 +58,70 @@ func (r *SessionRepo) UpsertSessionEdit(ctx context.Context, edit *domain.Sessio return nil } +// MarkSessionEdit upserts only the correctness mark for a session row, +// preserving any existing content/tag overlay. On first mark it inserts a +// mark-only row (edited_content = ” sentinel, original_content snapshot); +// on re-mark it updates correctness and bumps version without touching the +// content overlay. +func (r *SessionRepo) MarkSessionEdit(ctx context.Context, edit *domain.SessionEdit) error { + if edit == nil || edit.ID == "" { + return fmt.Errorf("session edit: id required") + } + state := edit.State + if state == "" { + state = domain.StateActive + } + _, err := r.db.ExecContext(ctx, + `INSERT INTO session_edits + (id, app_id, session_id, seq, agent_id, original_content, edited_content, correctness, edited_by, version, state, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, '', ?, ?, 1, ?, NOW(), NOW()) + ON DUPLICATE KEY UPDATE + correctness = VALUES(correctness), + edited_by = VALUES(edited_by), + version = version + 1, + state = VALUES(state), + updated_at = NOW()`, + edit.ID, edit.AppID, nullStr(edit.SessionID), edit.Seq, nullStr(edit.AgentID), + edit.OriginalContent, nullStr(edit.Correctness), nullStr(edit.EditedBy), string(state), + ) + if err != nil { + return fmt.Errorf("session edit mark: %w", err) + } + return nil +} + +// ClearSessionEditMark removes the correctness mark. If the row carries no +// content override (edited_content = ” sentinel), the whole row is deleted +// so the turn returns to "no overlay record"; otherwise only the mark is +// cleared and the content overlay survives. Returns true if anything changed. +func (r *SessionRepo) ClearSessionEditMark(ctx context.Context, id string) (bool, error) { + // Delete mark-only rows outright (no content override to keep). + res, err := r.db.ExecContext(ctx, + `DELETE FROM session_edits WHERE id = ? AND edited_content = '' AND correctness IS NOT NULL`, id) + if err != nil { + if internaltenant.IsTableNotFoundError(err) { + return false, nil + } + return false, fmt.Errorf("session edit clear mark (delete): %w", err) + } + if n, _ := res.RowsAffected(); n > 0 { + return true, nil + } + // Content overlay present: keep the row, just clear the mark. + res, err = r.db.ExecContext(ctx, + `UPDATE session_edits SET correctness = NULL, version = version + 1, updated_at = NOW() + WHERE id = ? AND correctness IS NOT NULL`, id) + if err != nil { + return false, fmt.Errorf("session edit clear mark (update): %w", err) + } + n, _ := res.RowsAffected() + return n > 0, nil +} + // GetSessionEdit returns the active overlay for id, or domain.ErrNotFound. func (r *SessionRepo) GetSessionEdit(ctx context.Context, id string) (*domain.SessionEdit, error) { row := r.db.QueryRowContext(ctx, - `SELECT id, app_id, session_id, seq, agent_id, original_content, edited_content, edited_tags, edited_by, reason, version, state, created_at, updated_at + `SELECT id, app_id, session_id, seq, agent_id, original_content, edited_content, edited_tags, correctness, edited_by, reason, version, state, created_at, updated_at FROM session_edits WHERE id = ? AND state = 'active'`, id, ) @@ -92,7 +153,7 @@ func (r *SessionRepo) GetSessionEditsByIDs(ctx context.Context, ids []string) (m args[i] = id } rows, err := r.db.QueryContext(ctx, - `SELECT id, app_id, session_id, seq, agent_id, original_content, edited_content, edited_tags, edited_by, reason, version, state, created_at, updated_at + `SELECT id, app_id, session_id, seq, agent_id, original_content, edited_content, edited_tags, correctness, edited_by, reason, version, state, created_at, updated_at FROM session_edits WHERE state = 'active' AND id IN (`+strings.Join(placeholders, ",")+`)`, args..., ) @@ -136,29 +197,34 @@ type sessionEditScanner interface { func scanSessionEdit(s sessionEditScanner) (*domain.SessionEdit, error) { var ( - e domain.SessionEdit - sessionID sql.NullString - agentID sql.NullString - editedBy sql.NullString - reason sql.NullString - state sql.NullString - tagsJSON []byte - createdAt time.Time - updatedAt time.Time - seqNull sql.NullInt64 + e domain.SessionEdit + sessionID sql.NullString + agentID sql.NullString + correctness sql.NullString + editedBy sql.NullString + reason sql.NullString + state sql.NullString + tagsJSON []byte + createdAt time.Time + updatedAt time.Time + seqNull sql.NullInt64 ) if err := s.Scan( &e.ID, &e.AppID, &sessionID, &seqNull, &agentID, - &e.OriginalContent, &e.EditedContent, &tagsJSON, &editedBy, &reason, + &e.OriginalContent, &e.EditedContent, &tagsJSON, &correctness, &editedBy, &reason, &e.Version, &state, &createdAt, &updatedAt, ); err != nil { return nil, err } e.SessionID = sessionID.String e.AgentID = agentID.String + e.Correctness = correctness.String e.EditedBy = editedBy.String e.Reason = reason.String e.Seq = int(seqNull.Int64) + // edited_content = '' is the "no content override" sentinel (real edits + // always have non-empty content); only a non-empty value overrides. + e.EditedContentSet = e.EditedContent != "" // NULL edited_tags = no tag override; a non-NULL value (incl. "[]") is // an explicit override. if tagsJSON != nil { diff --git a/server/internal/service/session_edit.go b/server/internal/service/session_edit.go index fd6190f3..fde64096 100644 --- a/server/internal/service/session_edit.go +++ b/server/internal/service/session_edit.go @@ -32,16 +32,19 @@ func (s *SessionService) EditSessionOverlay( } edit := &domain.SessionEdit{ - ID: id, - AppID: base.AppID, - SessionID: base.SessionID, - Seq: sessionSeqFromMemoryMeta(base), - AgentID: base.AgentID, - OriginalContent: base.Content, - EditedContent: content, - EditedBy: editedBy, - Reason: reason, - State: domain.StateActive, + ID: id, + AppID: base.AppID, + SessionID: base.SessionID, + Seq: sessionSeqFromMemoryMeta(base), + AgentID: base.AgentID, + OriginalContent: base.Content, + EditedContent: content, + EditedContentSet: true, + // A content edit implies the turn has been corrected → mark correct. + Correctness: domain.SessionEditCorrect, + EditedBy: editedBy, + Reason: reason, + State: domain.StateActive, } // nil tags = "leave display tags unchanged"; a non-nil slice (incl. an // empty one) is an explicit override. @@ -77,6 +80,50 @@ func (s *SessionService) DeleteSessionOverlay(ctx context.Context, id string) (i return s.sessions.DeleteSessionEdit(ctx, id) } +// ErrInvalidCorrectness is returned for a mark value other than +// "correct" / "incorrect". +var ErrInvalidCorrectness = &domain.ValidationError{ + Field: "correctness", Message: "must be 'correct' or 'incorrect'", +} + +// MarkSessionOverlay sets the correctness mark on a session row without +// touching any content/tag overlay. It snapshots original_content and 404s +// when the session row is missing. Returns the stored overlay. +func (s *SessionService) MarkSessionOverlay(ctx context.Context, id, correctness, markedBy string) (*domain.SessionEdit, error) { + if correctness != domain.SessionEditCorrect && correctness != domain.SessionEditIncorrect { + return nil, ErrInvalidCorrectness + } + base, err := s.sessions.GetByID(ctx, id) + if err != nil { + return nil, err + } + edit := &domain.SessionEdit{ + ID: id, + AppID: base.AppID, + SessionID: base.SessionID, + Seq: sessionSeqFromMemoryMeta(base), + AgentID: base.AgentID, + OriginalContent: base.Content, + Correctness: correctness, + EditedBy: markedBy, + State: domain.StateActive, + } + if err := s.sessions.MarkSessionEdit(ctx, edit); err != nil { + return nil, err + } + return s.sessions.GetSessionEdit(ctx, id) +} + +// ClearSessionOverlayMark removes the correctness mark; if the row carried +// no content override it is deleted entirely. 404s when the session row is +// missing. Returns whether a mark was cleared. +func (s *SessionService) ClearSessionOverlayMark(ctx context.Context, id string) (bool, error) { + if _, err := s.sessions.GetByID(ctx, id); err != nil { + return false, err + } + return s.sessions.ClearSessionEditMark(ctx, id) +} + // ApplySessionOverlay rewrites session-type results in place: for every // memory whose id has an active overlay, the content/tags are replaced with // the edited versions and edited/edit_version/edited_at markers are merged @@ -110,34 +157,68 @@ func (s *SessionService) ApplySessionOverlay(ctx context.Context, memories []dom return memories } -// applyOverlayToMemory returns a copy of base with the overlay's edited -// content/tags applied and edit markers merged into metadata. +// SessionCorrectnessByIDs returns id -> correctness mark for the given +// session rows that have one. Used by raw-session browse to annotate turns +// with their review mark without applying the content overlay. A repository +// failure degrades to an empty map rather than failing the read. +func (s *SessionService) SessionCorrectnessByIDs(ctx context.Context, ids []string) map[string]string { + out := map[string]string{} + if len(ids) == 0 { + return out + } + overlays, err := s.sessions.GetSessionEditsByIDs(ctx, ids) + if err != nil { + return out + } + for id, ov := range overlays { + if ov.Correctness != "" { + out[id] = ov.Correctness + } + } + return out +} + +// applyOverlayToMemory returns a copy of base with the overlay applied: a +// content/tag override is rendered only when present, and edit / correctness +// markers are merged into metadata. A mark-only overlay (no content +// override) leaves content and tags untouched but still surfaces +// correctness. func applyOverlayToMemory(base domain.Memory, ov *domain.SessionEdit) domain.Memory { if ov == nil { return base } - base.Content = ov.EditedContent + // Only override rendered content when the overlay has a content edit + // (mark-only rows must leave the original content visible). + if ov.EditedContentSet { + base.Content = ov.EditedContent + } // Only override rendered tags when the edit explicitly set them; a // content-only edit must leave the original session tags intact. if ov.EditedTagsSet { base.Tags = ov.EditedTags } - base.Metadata = mergeEditMarkers(base.Metadata, ov) + base.Metadata = mergeOverlayMetadata(base.Metadata, ov) return base } -// mergeEditMarkers adds edited / edit_version / edited_at onto existing -// session metadata without dropping role/seq/content_type. -func mergeEditMarkers(existing json.RawMessage, ov *domain.SessionEdit) json.RawMessage { +// mergeOverlayMetadata merges overlay markers onto existing session metadata +// without dropping role/seq/content_type: edited / edit_version / edited_at +// when there is a content or tag override, and correctness when marked. +func mergeOverlayMetadata(existing json.RawMessage, ov *domain.SessionEdit) json.RawMessage { payload := map[string]any{} if len(existing) > 0 { if err := json.Unmarshal(existing, &payload); err != nil { payload = map[string]any{} } } - payload["edited"] = true - payload["edit_version"] = ov.Version - payload["edited_at"] = ov.UpdatedAt + if ov.EditedContentSet || ov.EditedTagsSet { + payload["edited"] = true + payload["edit_version"] = ov.Version + payload["edited_at"] = ov.UpdatedAt + } + if ov.Correctness != "" { + payload["correctness"] = ov.Correctness + } raw, err := json.Marshal(payload) if err != nil { return existing diff --git a/server/internal/service/session_edit_test.go b/server/internal/service/session_edit_test.go index 6b168bce..3d972dce 100644 --- a/server/internal/service/session_edit_test.go +++ b/server/internal/service/session_edit_test.go @@ -14,7 +14,7 @@ import ( func TestApplySessionOverlay_RewritesOnlyEditedSessions(t *testing.T) { repo := &stubSessionRepo{overlays: map[string]*domain.SessionEdit{ - "s1": {ID: "s1", EditedContent: "edited one", Version: 2, State: domain.StateActive}, + "s1": {ID: "s1", EditedContent: "edited one", EditedContentSet: true, Version: 2, State: domain.StateActive}, }} svc := newTestSessionService(repo) diff --git a/server/internal/service/session_test.go b/server/internal/service/session_test.go index 74adb851..2de2badf 100644 --- a/server/internal/service/session_test.go +++ b/server/internal/service/session_test.go @@ -184,6 +184,40 @@ func (s *stubSessionRepo) DeleteSessionEdit(_ context.Context, id string) (int64 return 0, nil } +func (s *stubSessionRepo) MarkSessionEdit(_ context.Context, edit *domain.SessionEdit) error { + if s.overlays == nil { + s.overlays = map[string]*domain.SessionEdit{} + } + if existing, ok := s.overlays[edit.ID]; ok { + existing.Correctness = edit.Correctness + existing.Version++ + return nil + } + cp := *edit + cp.EditedContent = "" + cp.EditedContentSet = false + cp.Version = 1 + if cp.State == "" { + cp.State = domain.StateActive + } + s.overlays[edit.ID] = &cp + return nil +} + +func (s *stubSessionRepo) ClearSessionEditMark(_ context.Context, id string) (bool, error) { + ov, ok := s.overlays[id] + if !ok || ov.Correctness == "" { + return false, nil + } + if !ov.EditedContentSet { + delete(s.overlays, id) + return true, nil + } + ov.Correctness = "" + ov.Version++ + return true, nil +} + func newTestSessionService(repo *stubSessionRepo) *SessionService { return NewSessionService(repo, nil, "") } @@ -662,3 +696,9 @@ func (c *capturingSessionRepo) GetSessionEditsByIDs(ctx context.Context, ids []s func (c *capturingSessionRepo) DeleteSessionEdit(ctx context.Context, id string) (int64, error) { return c.stub.DeleteSessionEdit(ctx, id) } +func (c *capturingSessionRepo) MarkSessionEdit(ctx context.Context, edit *domain.SessionEdit) error { + return c.stub.MarkSessionEdit(ctx, edit) +} +func (c *capturingSessionRepo) ClearSessionEditMark(ctx context.Context, id string) (bool, error) { + return c.stub.ClearSessionEditMark(ctx, id) +} diff --git a/server/internal/tenant/provisioner_test.go b/server/internal/tenant/provisioner_test.go index 43137a84..f23e5c74 100644 --- a/server/internal/tenant/provisioner_test.go +++ b/server/internal/tenant/provisioner_test.go @@ -142,6 +142,9 @@ func (c *schemaInitConnector) recordDDL(query string) { case strings.Contains(lowerQuery, "create table if not exists session_edits"): c.existingTables["session_edits"] = true c.existingCols["session_edits.edited_content"] = true + c.existingCols["session_edits.correctness"] = true + case strings.Contains(lowerQuery, "alter table session_edits add column correctness"): + c.existingCols["session_edits.correctness"] = true case strings.Contains(lowerQuery, "create table if not exists sessions"): c.existingTables["sessions"] = true c.existingCols["sessions.app_id"] = true diff --git a/server/internal/tenant/schema.go b/server/internal/tenant/schema.go index d5a3b934..8466ad86 100644 --- a/server/internal/tenant/schema.go +++ b/server/internal/tenant/schema.go @@ -165,6 +165,11 @@ const TenantSessionsSchemaBase = `CREATE TABLE IF NOT EXISTS sessions ( // only affects how Session Search renders an already-matched row — it does // not change what is searchable, and memory/fact recall is untouched. // No embedding column: the overlay never participates in retrieval. +// A row can carry a content/tag edit, a correctness mark, or both: +// - edited_content = ” means "no content override" (real edits always +// have non-empty content, so ” is an unambiguous sentinel); +// - correctness ('correct' | 'incorrect') is a human review mark, NULL +// when unmarked. A content edit auto-sets it to 'correct'. const TenantSessionEditsSchema = `CREATE TABLE IF NOT EXISTS session_edits ( id VARCHAR(36) PRIMARY KEY, app_id VARCHAR(100) NOT NULL DEFAULT '', @@ -174,6 +179,7 @@ const TenantSessionEditsSchema = `CREATE TABLE IF NOT EXISTS session_edits ( original_content MEDIUMTEXT NOT NULL, edited_content MEDIUMTEXT NOT NULL, edited_tags JSON, + correctness VARCHAR(20) NULL, edited_by VARCHAR(100) NULL, reason VARCHAR(500) NULL, version INT NOT NULL DEFAULT 1, @@ -262,6 +268,12 @@ func ensureTiDBTenantTablesAndSearchIndexes(ctx context.Context, db *sql.DB, aut if err := ensureTable(ctx, db, "session_edits", TenantSessionEditsSchema); err != nil { return fmt.Errorf("session_edits table: %w", err) } + // correctness column was added after the table's first release; migrate + // existing tenants in place. + if err := ensureColumn(ctx, db, "session_edits", "correctness", + `ALTER TABLE session_edits ADD COLUMN correctness VARCHAR(20) NULL`); err != nil { + return fmt.Errorf("session_edits correctness column: %w", err) + } return nil } @@ -489,6 +501,24 @@ func ensureTable(ctx context.Context, db *sql.DB, table, createSQL string) error return nil } +// ensureColumn adds a column to an existing table if it is missing. Used to +// migrate tables created by earlier releases (CREATE TABLE IF NOT EXISTS is +// a no-op once the table exists, so new columns need an explicit ALTER). +// alterSQL must be the full "ALTER TABLE ... ADD COLUMN ..." statement. +func ensureColumn(ctx context.Context, db *sql.DB, table, column, alterSQL string) error { + exists, err := ColumnExists(ctx, db, table, column) + if err != nil { + return fmt.Errorf("check column: %w", err) + } + if exists { + return nil + } + if _, err := db.ExecContext(ctx, alterSQL); err != nil { + return fmt.Errorf("add column: %w", err) + } + return nil +} + func ensureVectorIndex(ctx context.Context, db *sql.DB, table, indexName string) error { exists, err := IndexExists(ctx, db, table, indexName) if err != nil { diff --git a/server/schema.sql b/server/schema.sql index 3774dbb5..5e5f9aef 100644 --- a/server/schema.sql +++ b/server/schema.sql @@ -175,8 +175,9 @@ CREATE TABLE IF NOT EXISTS session_edits ( seq INT NULL, agent_id VARCHAR(100) NULL, original_content MEDIUMTEXT NOT NULL COMMENT 'Pre-edit snapshot (before)', - edited_content MEDIUMTEXT NOT NULL COMMENT 'Post-edit content (after)', + edited_content MEDIUMTEXT NOT NULL COMMENT 'Post-edit content; empty string = no content override (mark-only row)', edited_tags JSON NULL COMMENT 'NULL = no tag override; non-NULL (incl. []) overrides', + correctness VARCHAR(20) NULL COMMENT 'Review mark: correct|incorrect; NULL = unmarked. A content edit auto-sets correct', edited_by VARCHAR(100) NULL, reason VARCHAR(500) NULL, version INT NOT NULL DEFAULT 1,