From 31086ed1ab9dc3d484a5ce4fb85b28b14272ae8c Mon Sep 17 00:00:00 2001 From: AskaEth Date: Sat, 3 Oct 2026 16:08:50 +0800 Subject: [PATCH] =?UTF-8?q?fix(memory):=20=E5=AE=9A=E5=AE=BD=E6=97=B6?= =?UTF-8?q?=E9=97=B4=E6=88=B3=E5=AD=98=E5=82=A8=20+=20rowid=20=E7=A1=AE?= =?UTF-8?q?=E5=AE=9A=E6=80=A7=E6=8E=92=E5=BA=8F=20+=20=E9=81=97=E7=95=99?= =?UTF-8?q?=E6=95=B0=E6=8D=AE=E5=BD=92=E4=B8=80=E5=8C=96?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - formatTime 改用固定 30 字节布局(9 位小数秒)，让 TEXT 列的字典序与时间序一致（RFC3339Nano 会去掉尾随零产生变宽字符串，秒内会反序） - memory_items / project / project_chapter 的 ORDER BY 全部补 rowid 兜底，消除同 tick 打平时的顺序不确定 - 开库时幂等归一化变宽的历史值；无法解析的行原样保留，不阻塞开库 - 回归测试：变宽排序反例、同一时间戳插入序、归一化幂等与跳过 --- .../runtime/memory/projectindex/chapter.go | 8 +- .../runtime/memory/projectindex/project.go | 18 -- internal/runtime/memory/projectindex/store.go | 16 +- .../runtime/memory/projectindex/timefmt.go | 141 +++++++++++++ .../memory/projectindex/timefmt_test.go | 187 ++++++++++++++++++ internal/runtime/memory/rules.go | 10 +- internal/runtime/memory/store.go | 52 ++--- internal/runtime/memory/store_test.go | 91 +++++++++ internal/runtime/memory/timefmt.go | 135 +++++++++++++ internal/runtime/memory/timefmt_test.go | 148 ++++++++++++++ 10 files changed, 745 insertions(+), 61 deletions(-) create mode 100644 internal/runtime/memory/projectindex/timefmt.go create mode 100644 internal/runtime/memory/projectindex/timefmt_test.go create mode 100644 internal/runtime/memory/timefmt.go create mode 100644 internal/runtime/memory/timefmt_test.go diff --git a/internal/runtime/memory/projectindex/chapter.go b/internal/runtime/memory/projectindex/chapter.go index f5c49d9..ac2b725 100644 --- a/internal/runtime/memory/projectindex/chapter.go +++ b/internal/runtime/memory/projectindex/chapter.go @@ -61,8 +61,10 @@ func (s *Store) AddChapter(ctx context.Context, projectID string, ch Chapter) er return nil } -// ListChapters returns every chapter for projectID in insertion order. It returns -// an empty (non-nil) slice when there are none; a blank projectID returns +// ListChapters returns every chapter for projectID in insertion order +// (created_at ASC, rowid ASC) — rowid breaks created_at ties so chapters written +// within the same clock tick keep their insertion order. It returns an empty +// (non-nil) slice when there are none; a blank projectID returns // ErrMissingProject. func (s *Store) ListChapters(ctx context.Context, projectID string) ([]Chapter, error) { if strings.TrimSpace(projectID) == "" { @@ -70,7 +72,7 @@ func (s *Store) ListChapters(ctx context.Context, projectID string) ([]Chapter, } rows, err := s.db.QueryContext(ctx, `SELECT id, project_id, title, content, created_at, updated_at FROM project_chapter - WHERE project_id = ? ORDER BY created_at ASC, id ASC`, projectID) + WHERE project_id = ? ORDER BY created_at ASC, rowid ASC`, projectID) if err != nil { return nil, fmt.Errorf("projectindex: list chapters: %w", err) } diff --git a/internal/runtime/memory/projectindex/project.go b/internal/runtime/memory/projectindex/project.go index 0126df6..4c32c4b 100644 --- a/internal/runtime/memory/projectindex/project.go +++ b/internal/runtime/memory/projectindex/project.go @@ -142,21 +142,3 @@ func newID() (string, error) { } return hex.EncodeToString(b), nil } - -// formatTime encodes t in UTC as an RFC3339Nano string so ordering and -// round-trips are stable across processes. -func formatTime(t time.Time) string { - return t.UTC().Format(time.RFC3339Nano) -} - -// parseTime decodes a stored RFC3339 string; blank input yields a zero time. -func parseTime(s string) (time.Time, error) { - if strings.TrimSpace(s) == "" { - return time.Time{}, nil - } - t, err := time.Parse(time.RFC3339Nano, s) - if err != nil { - return time.Time{}, fmt.Errorf("projectindex: parse time %q: %w", s, err) - } - return t, nil -} diff --git a/internal/runtime/memory/projectindex/store.go b/internal/runtime/memory/projectindex/store.go index b0b660c..f59a4d3 100644 --- a/internal/runtime/memory/projectindex/store.go +++ b/internal/runtime/memory/projectindex/store.go @@ -79,7 +79,8 @@ type rowScanner interface { // internal/db). A nil db returns ErrNilDB. log may be nil, in which case a // discard logger is used; the active logger is tagged with source // "memory/projectindex" so every record is traceable. ctx controls schema -// creation and must be honored by the caller. +// creation and must be honored by the caller. Legacy variable-width timestamps +// are normalized in place (see normalizeTimestamps). func New(ctx context.Context, db *sql.DB, log *slog.Logger) (*Store, error) { if db == nil { return nil, ErrNilDB @@ -94,6 +95,9 @@ func New(ctx context.Context, db *sql.DB, log *slog.Logger) (*Store, error) { return nil, fmt.Errorf("projectindex: apply schema: %w", err) } } + if err := normalizeTimestamps(ctx, db); err != nil { + return nil, err + } return &Store{db: db, log: log}, nil } @@ -106,12 +110,14 @@ func discardLogger(log *slog.Logger) *slog.Logger { } // ListProjects returns every project index entry in insertion order (created_at -// ASC, id ASC). An empty (non-nil) slice is returned when there are none, so -// callers can range over it unconditionally. A read error is wrapped with %w so -// callers can use errors.Is against the package sentinels. +// ASC, rowid ASC) — rowid breaks created_at ties so projects written within the +// same clock tick keep their insertion order. An empty (non-nil) slice is +// returned when there are none, so callers can range over it unconditionally. A +// read error is wrapped with %w so callers can use errors.Is against the package +// sentinels. func (s *Store) ListProjects(ctx context.Context) ([]Project, error) { rows, err := s.db.QueryContext(ctx, `SELECT id, title, desc, created_at, updated_at - FROM project ORDER BY created_at ASC, id ASC`) + FROM project ORDER BY created_at ASC, rowid ASC`) if err != nil { return nil, fmt.Errorf("projectindex: list projects: %w", err) } diff --git a/internal/runtime/memory/projectindex/timefmt.go b/internal/runtime/memory/projectindex/timefmt.go new file mode 100644 index 0000000..053c8c5 --- /dev/null +++ b/internal/runtime/memory/projectindex/timefmt.go @@ -0,0 +1,141 @@ +package projectindex + +import ( + "context" + "database/sql" + "fmt" + "strings" + "time" +) + +// timeLayout is the fixed-width timestamp layout used for every stored +// created_at/updated_at value. Unlike time.RFC3339Nano it never drops trailing +// fractional zeros, so every value is exactly storedTimeLen bytes wide; SQLite +// compares these TEXT columns as plain strings and equal width is what makes +// that comparison agree with chronological order. UTC values render the zone as +// a single "Z". +const timeLayout = "2006-01-02T15:04:05.000000000Z07:00" + +// storedTimeLen is the byte length of a UTC value formatted with timeLayout: +// "2006-01-02T15:04:05.000000000Z" — the Z07:00 zone collapses to "Z" in UTC. +const storedTimeLen = 30 + +// Table names normalized on open; they must match the DDL in store.go. +const ( + projectTable = "project" + projectChapterTable = "project_chapter" +) + +// formatTime encodes t in UTC with the fixed-width timeLayout. Always writing +// UTC keeps ordering and round-trips stable across processes, and the fixed +// width is what lets ORDER BY created_at order these TEXT columns by time. +func formatTime(t time.Time) string { + return t.UTC().Format(timeLayout) +} + +// parseTime decodes a stored timestamp. time.RFC3339Nano accepts both the +// fixed-width values this package writes and the legacy variable-width ones, so +// the same parser serves reads and the open-time normalization. Blank input +// yields a zero time. +func parseTime(s string) (time.Time, error) { + if strings.TrimSpace(s) == "" { + return time.Time{}, nil + } + t, err := time.Parse(time.RFC3339Nano, s) + if err != nil { + return time.Time{}, fmt.Errorf("projectindex: parse time %q: %w", s, err) + } + return t, nil +} + +// normalizeTimestamps rewrites every project / project_chapter timestamp that is +// not storedTimeLen bytes wide into the fixed-width timeLayout. Rows written +// before the layout existed carry variable-width RFC3339Nano text whose string +// order is not its time order; this brings them in line. +// +// It is idempotent: only rows whose column length differs from storedTimeLen are +// selected, so running it again on a normalized table is a no-op. A row whose +// value does not parse is left exactly as it is — one bad legacy value must not +// keep the store from opening. +func normalizeTimestamps(ctx context.Context, db *sql.DB) error { + for _, table := range []string{projectTable, projectChapterTable} { + for _, col := range []string{"created_at", "updated_at"} { + if err := normalizeTimeColumn(ctx, db, table, col); err != nil { + return err + } + } + } + return nil +} + +// normalizeTimeColumn rewrites every table.col value that is not storedTimeLen +// bytes wide. table and col are compile-time constants, never user input, so +// splicing them into the statement is safe; the length comparison and every +// value written are parameterized. All rewrites are collected before any UPDATE +// runs, so no write happens while the SELECT cursor is still open. +func normalizeTimeColumn(ctx context.Context, db *sql.DB, table, col string) error { + fixes, err := collectLegacyTimestamps(ctx, db, table, col) + if err != nil { + return err + } + for _, f := range fixes { + if _, err := db.ExecContext(ctx, + "UPDATE "+f.table+" SET "+f.col+" = ? WHERE id = ?", f.value, f.id); err != nil { + return fmt.Errorf("projectindex: normalize %s.%s: %w", f.table, f.col, err) + } + } + return nil +} + +// timeFix is one pending timestamp rewrite: which table and column, which row +// (by primary key id), and the fixed-width value to write. +type timeFix struct { + table string + col string + id string + value string +} + +// collectLegacyTimestamps reads the rows of table.col whose value is not +// storedTimeLen bytes wide and returns the parseable ones as pending rewrites. +// Unparseable values are skipped, so normalization can never fail a row: the +// only errors are genuine read failures. +func collectLegacyTimestamps(ctx context.Context, db *sql.DB, table, col string) ([]timeFix, error) { + rows, err := db.QueryContext(ctx, + "SELECT id, "+col+" FROM "+table+" WHERE length("+col+") != ?", storedTimeLen) + if err != nil { + return nil, fmt.Errorf("projectindex: query legacy %s.%s: %w", table, col, err) + } + defer func() { _ = rows.Close() }() + + fixes := make([]timeFix, 0) + for rows.Next() { + var id, raw string + if err := rows.Scan(&id, &raw); err != nil { + return nil, fmt.Errorf("projectindex: scan legacy %s.%s: %w", table, col, err) + } + fixed, ok := reformatTime(raw) + if !ok { + continue + } + fixes = append(fixes, timeFix{table: table, col: col, id: id, value: fixed}) + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("projectindex: iterate legacy %s.%s: %w", table, col, err) + } + return fixes, nil +} + +// reformatTime re-encodes raw with timeLayout. The bool is false when raw is +// already storedTimeLen wide or when it does not parse, in which case the caller +// leaves the stored value untouched. +func reformatTime(raw string) (string, bool) { + if len(raw) == storedTimeLen { + return "", false + } + t, err := parseTime(raw) + if err != nil || t.IsZero() { + return "", false + } + return formatTime(t), true +} diff --git a/internal/runtime/memory/projectindex/timefmt_test.go b/internal/runtime/memory/projectindex/timefmt_test.go new file mode 100644 index 0000000..617e65e --- /dev/null +++ b/internal/runtime/memory/projectindex/timefmt_test.go @@ -0,0 +1,187 @@ +package projectindex + +import ( + "context" + "testing" + "time" +) + +// insertLegacyChapterSQL writes a project_chapter row with caller-supplied raw +// timestamps, standing in for a row persisted before the fixed-width layout. +const insertLegacyChapterSQL = `INSERT INTO project_chapter + (id, project_id, title, content, created_at, updated_at) + VALUES (?, 'proj.1', '旧章节', '旧内容', ?, ?)` + +func TestFormatTime_定宽_正常(t *testing.T) { + t.Parallel() + tests := []struct { + name string + at time.Time + want string + }{ + {"一位小数秒", time.Date(2026, 9, 7, 3, 20, 5, 830000000, time.UTC), "2026-09-07T03:20:05.830000000Z"}, + {"纳秒", time.Date(2026, 9, 7, 3, 20, 5, 830978900, time.UTC), "2026-09-07T03:20:05.830978900Z"}, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := formatTime(tt.at) + if got != tt.want { + t.Errorf("formatTime = %q, want %q", got, tt.want) + } + if len(got) != storedTimeLen { + t.Errorf("formatTime length = %d, want %d", len(got), storedTimeLen) + } + }) + } +} + +// TestStore_ListChapters_变宽时间戳_排序 is the projectindex regression for the +// variable-width timestamp bug: RFC3339Nano would store these as ":05.83" and +// ":05.8309789", whose text order is the reverse of their time order. +func TestStore_ListChapters_变宽时间戳_排序(t *testing.T) { + t.Parallel() + ctx := context.Background() + s := newTestStore(t) + + sec := time.Date(2026, 9, 7, 3, 20, 5, 0, time.UTC) + older := sec.Add(830 * time.Millisecond) + newer := older.Add(978900 * time.Nanosecond) + + // The newer chapter is inserted first, so a rowid-only tiebreak would put it + // last: only correctly ordered created_at text places it last-but-one. + if err := s.AddChapter(ctx, "proj.1", Chapter{ + Title: "较晚", Content: "x", CreatedAt: newer, UpdatedAt: newer, + }); err != nil { + t.Fatalf("AddChapter(较晚): %v", err) + } + if err := s.AddChapter(ctx, "proj.1", Chapter{ + Title: "较早", Content: "y", CreatedAt: older, UpdatedAt: older, + }); err != nil { + t.Fatalf("AddChapter(较早): %v", err) + } + + var raw string + if err := s.db.QueryRowContext(ctx, + `SELECT created_at FROM project_chapter WHERE title = ?`, "较早").Scan(&raw); err != nil { + t.Fatalf("read stored created_at: %v", err) + } + if len(raw) != storedTimeLen { + t.Errorf("stored created_at = %q (%d bytes), want fixed width %d", raw, len(raw), storedTimeLen) + } + + list, err := s.ListChapters(ctx, "proj.1") + if err != nil { + t.Fatalf("ListChapters: %v", err) + } + if len(list) != 2 { + t.Fatalf("ListChapters = %d chapters, want 2", len(list)) + } + if list[0].Title != "较早" || list[1].Title != "较晚" { + t.Errorf("titles = [%q %q], want oldest first (较早, 较晚)", list[0].Title, list[1].Title) + } +} + +// TestStore_ListChapters_同一时间戳_插入序 covers the tie case: consecutive +// AddChapter calls can share one clock tick, so rowid ASC must keep them in +// insertion order. +func TestStore_ListChapters_同一时间戳_插入序(t *testing.T) { + t.Parallel() + ctx := context.Background() + s := newTestStore(t) + + at := time.Date(2026, 9, 7, 3, 20, 5, 830978900, time.UTC) + for _, title := range []string{"第一章", "第二章", "第三章"} { + if err := s.AddChapter(ctx, "proj.1", Chapter{ + Title: title, Content: "内容", CreatedAt: at, UpdatedAt: at, + }); err != nil { + t.Fatalf("AddChapter(%s): %v", title, err) + } + } + + list, err := s.ListChapters(ctx, "proj.1") + if err != nil { + t.Fatalf("ListChapters: %v", err) + } + want := []string{"第一章", "第二章", "第三章"} + if len(list) != len(want) { + t.Fatalf("ListChapters = %d chapters, want %d", len(list), len(want)) + } + for i, w := range want { + if list[i].Title != w { + t.Errorf("chapters[%d].Title = %q, want %q (rowid ASC)", i, list[i].Title, w) + } + } +} + +// TestNew_归一化历史时间戳_正常 writes legacy variable-width rows into both +// tables, re-opens the store to trigger normalization, and checks the repaired +// text plus idempotence and the unparseable-row skip. +func TestNew_归一化历史时间戳_正常(t *testing.T) { + t.Parallel() + ctx := context.Background() + s := newTestStore(t) + + const legacy = "2026-09-07T03:20:05.83Z" + if _, err := s.db.ExecContext(ctx, `INSERT INTO project (id, title, desc, created_at, updated_at) + VALUES ('proj.legacy', 'T', '', ?, ?)`, legacy, legacy); err != nil { + t.Fatalf("insert legacy project: %v", err) + } + if _, err := s.db.ExecContext(ctx, insertLegacyChapterSQL, "ch.legacy", legacy, legacy); err != nil { + t.Fatalf("insert legacy chapter: %v", err) + } + if _, err := s.db.ExecContext(ctx, insertLegacyChapterSQL, "ch.broken", "not-a-time", "not-a-time"); err != nil { + t.Fatalf("insert unparseable chapter: %v", err) + } + + // Re-opening normalizes both tables; an unparseable value must not block it. + if _, err := New(ctx, s.db, testLogger()); err != nil { + t.Fatalf("New (normalize): %v", err) + } + + const want = "2026-09-07T03:20:05.830000000Z" + for _, tc := range []struct{ name, query, id string }{ + {"project", `SELECT created_at FROM project WHERE id = ?`, "proj.legacy"}, + {"project_chapter", `SELECT created_at FROM project_chapter WHERE id = ?`, "ch.legacy"}, + } { + var raw string + if err := s.db.QueryRowContext(ctx, tc.query, tc.id).Scan(&raw); err != nil { + t.Fatalf("read %s created_at: %v", tc.name, err) + } + if raw != want { + t.Errorf("%s created_at = %q, want %q", tc.name, raw, want) + } + } + + var broken string + if err := s.db.QueryRowContext(ctx, + `SELECT created_at FROM project_chapter WHERE id = ?`, "ch.broken").Scan(&broken); err != nil { + t.Fatalf("read unparseable created_at: %v", err) + } + if broken != "not-a-time" { + t.Errorf("unparseable created_at = %q, want it left as is", broken) + } + + // A further open must be a no-op. + if _, err := New(ctx, s.db, testLogger()); err != nil { + t.Fatalf("New (second pass): %v", err) + } + var again string + if err := s.db.QueryRowContext(ctx, + `SELECT created_at FROM project WHERE id = ?`, "proj.legacy").Scan(&again); err != nil { + t.Fatalf("read created_at after second open: %v", err) + } + if again != want { + t.Errorf("second open changed created_at: %q -> %q", want, again) + } + + got, err := s.GetProject(ctx, "proj.legacy") + if err != nil { + t.Fatalf("GetProject: %v", err) + } + wantTime := time.Date(2026, 9, 7, 3, 20, 5, 830000000, time.UTC) + if !got.CreatedAt.Equal(wantTime) { + t.Errorf("project created_at = %v, want %v", got.CreatedAt, wantTime) + } +} diff --git a/internal/runtime/memory/rules.go b/internal/runtime/memory/rules.go index a6a2e40..2aebe83 100644 --- a/internal/runtime/memory/rules.go +++ b/internal/runtime/memory/rules.go @@ -61,7 +61,8 @@ func (s *Store) AddScoped(ctx context.Context, actorID string, item MemoryItem) } // ListScope returns every item of scope visible to personaID, newest first, -// applying the design §3 read rules: +// applying the design §3 read rules (rowid breaks created_at ties so items +// written within the same clock tick keep a stable newest-first order): // // - ScopeUser: every user-scope item (globally shared across personas). // - ScopePersona: every persona-scope item. Other personas are read-only but @@ -82,14 +83,15 @@ func (s *Store) ListScope(ctx context.Context, scope Scope, personaID string) ([ rows, err := s.db.QueryContext(ctx, `SELECT id, persona_id, role_id, project_id, scope, kind, content, tags, confidence, source_conversation, created_at, updated_at, decay_weight FROM memory_items WHERE scope = ? - ORDER BY created_at DESC`, string(scope)) + ORDER BY created_at DESC, rowid DESC`, string(scope)) if err != nil { return nil, fmt.Errorf("memory: list scope items: %w", err) } return scanItems(rows, "scope item") } -// ListProject returns every memory item linked to projectID, newest first. A +// ListProject returns every memory item linked to projectID, newest first, with +// rowid breaking created_at ties so same-tick writes keep a stable order. A // project-scoped memory carries a ProjectID that links to the projectindex // hierarchy (design §3); this is the deep-search entry point — callers combine // it with the projectindex store to walk 项目索引 → 项目描述 → 章节内容. Project @@ -102,7 +104,7 @@ func (s *Store) ListProject(ctx context.Context, projectID string) ([]MemoryItem rows, err := s.db.QueryContext(ctx, `SELECT id, persona_id, role_id, project_id, scope, kind, content, tags, confidence, source_conversation, created_at, updated_at, decay_weight FROM memory_items WHERE project_id = ? - ORDER BY created_at DESC`, projectID) + ORDER BY created_at DESC, rowid DESC`, projectID) if err != nil { return nil, fmt.Errorf("memory: list project items: %w", err) } diff --git a/internal/runtime/memory/store.go b/internal/runtime/memory/store.go index acd2911..b2e29b1 100644 --- a/internal/runtime/memory/store.go +++ b/internal/runtime/memory/store.go @@ -158,7 +158,8 @@ type rowScanner interface { // db must be a live, migrated connection (the migration framework itself is // owned by internal/db). A nil db returns ErrNilDB. log may be nil, in which // case a discard logger is used. ctx controls schema creation and must be -// honored by the caller. +// honored by the caller. Any legacy variable-width timestamps are normalized in +// place (see normalizeMemoryTimestamps), so text ordering matches time ordering. func NewStore(ctx context.Context, db *sql.DB, log *slog.Logger) (*Store, error) { if db == nil { return nil, ErrNilDB @@ -176,6 +177,9 @@ func NewStore(ctx context.Context, db *sql.DB, log *slog.Logger) (*Store, error) if err := ensureProjectIDColumn(ctx, db); err != nil { return nil, err } + if err := normalizeMemoryTimestamps(ctx, db); err != nil { + return nil, err + } return &Store{db: db, log: log}, nil } @@ -227,11 +231,12 @@ func (s *Store) Get(ctx context.Context, id string) (MemoryItem, error) { return item, nil } -// ListByPersona returns every item for personaID, newest first. It is the -// persona-isolation boundary: items belonging to other personas are never -// returned, so personas never share memory (design §4). Use ListScope when a -// caller needs to see shared user/project scopes or another persona's read-only -// items. +// ListByPersona returns every item for personaID, newest first. rowid breaks +// created_at ties so items written within the same clock tick keep a stable +// newest-first order. It is the persona-isolation boundary: items belonging to +// other personas are never returned, so personas never share memory (design +// §4). Use ListScope when a caller needs to see shared user/project scopes or +// another persona's read-only items. func (s *Store) ListByPersona(ctx context.Context, personaID string) ([]MemoryItem, error) { if strings.TrimSpace(personaID) == "" { return nil, ErrMissingPersona @@ -239,22 +244,24 @@ func (s *Store) ListByPersona(ctx context.Context, personaID string) ([]MemoryIt rows, err := s.db.QueryContext(ctx, `SELECT id, persona_id, role_id, project_id, scope, kind, content, tags, confidence, source_conversation, created_at, updated_at, decay_weight FROM memory_items WHERE persona_id = ? - ORDER BY created_at DESC`, personaID) + ORDER BY created_at DESC, rowid DESC`, personaID) if err != nil { return nil, fmt.Errorf("memory: list persona items: %w", err) } return scanItems(rows, "persona item") } -// List returns every memory item, newest first, across all personas. It is the -// aggregate/admin view that enumerates all persisted memory regardless of the -// persona-isolation boundary; callers that must respect isolation should use -// ListByPersona or ListScope instead. The returned slice is non-nil. +// List returns every memory item, newest first, across all personas. rowid +// breaks created_at ties so items written within the same clock tick keep a +// stable newest-first order. It is the aggregate/admin view that enumerates all +// persisted memory regardless of the persona-isolation boundary; callers that +// must respect isolation should use ListByPersona or ListScope instead. The +// returned slice is non-nil. func (s *Store) List(ctx context.Context) ([]MemoryItem, error) { rows, err := s.db.QueryContext(ctx, `SELECT id, persona_id, role_id, project_id, scope, kind, content, tags, confidence, source_conversation, created_at, updated_at, decay_weight FROM memory_items - ORDER BY created_at DESC`) + ORDER BY created_at DESC, rowid DESC`) if err != nil { return nil, fmt.Errorf("memory: list items: %w", err) } @@ -378,7 +385,8 @@ func applyTimestamps(item MemoryItem) (MemoryItem, error) { } // scanItem reads one memory_items row. tags is stored as a JSON array string; -// timestamps are stored as RFC3339 strings. +// timestamps are stored as fixed-width UTC strings (timeLayout) so text +// comparison orders them chronologically. func scanItem(sc rowScanner) (MemoryItem, error) { var ( it MemoryItem @@ -421,24 +429,6 @@ func newID() (string, error) { return hex.EncodeToString(b), nil } -// formatTime encodes t in UTC as an RFC3339 string. The value is always written -// as UTC so ordering and round-trips are stable across processes. -func formatTime(t time.Time) string { - return t.UTC().Format(time.RFC3339Nano) -} - -// parseTime decodes a stored RFC3339 string. Blank input yields a zero time. -func parseTime(s string) (time.Time, error) { - if strings.TrimSpace(s) == "" { - return time.Time{}, nil - } - t, err := time.Parse(time.RFC3339Nano, s) - if err != nil { - return time.Time{}, fmt.Errorf("memory: parse time %q: %w", s, err) - } - return t, nil -} - // discardLogger returns log if non-nil, otherwise a logger that writes nowhere. func discardLogger(log *slog.Logger) *slog.Logger { if log != nil { diff --git a/internal/runtime/memory/store_test.go b/internal/runtime/memory/store_test.go index b219659..0a84630 100644 --- a/internal/runtime/memory/store_test.go +++ b/internal/runtime/memory/store_test.go @@ -5,6 +5,7 @@ import ( "errors" "sync" "testing" + "time" ) func TestNewStore_NilDB_异常(t *testing.T) { @@ -210,6 +211,96 @@ func TestStore_List_正常(t *testing.T) { } } +// TestStore_List_变宽时间戳_排序 is the regression for the variable-width +// timestamp bug: time.RFC3339Nano drops trailing fractional zeros, so the older +// item was once stored as ":05.83" and the newer as ":05.8309789", and the text +// comparison SQLite applies to these columns ranked the older one as newer. +func TestStore_List_变宽时间戳_排序(t *testing.T) { + t.Parallel() + ctx := context.Background() + s := newTestStore(t) + + sec := time.Date(2026, 9, 7, 3, 20, 5, 0, time.UTC) + older := sec.Add(830 * time.Millisecond) // ":05.83" under RFC3339Nano + newer := older.Add(978900 * time.Nanosecond) // ":05.8309789" under RFC3339Nano + + // The newer item is inserted first, so a rowid-only tiebreak would surface + // the older one: only correctly ordered created_at text yields 较晚 first. + for _, in := range []struct { + content string + at time.Time + }{{"较晚", newer}, {"较早", older}} { + item := MemoryItem{PersonaID: "persona.a", Content: in.content, CreatedAt: in.at, UpdatedAt: in.at} + if _, err := s.Add(ctx, item); err != nil { + t.Fatalf("Add(%s): %v", in.content, err) + } + } + + var raw string + if err := s.db.QueryRowContext(ctx, + `SELECT created_at FROM memory_items WHERE content = ?`, "较早").Scan(&raw); err != nil { + t.Fatalf("read stored created_at: %v", err) + } + if len(raw) != storedTimeLen { + t.Errorf("stored created_at = %q (%d bytes), want fixed width %d", raw, len(raw), storedTimeLen) + } + + got, err := s.List(ctx) + if err != nil { + t.Fatalf("List: %v", err) + } + if len(got) != 2 { + t.Fatalf("List = %d items, want 2", len(got)) + } + if got[0].Content != "较晚" { + t.Errorf("got[0].Content = %q, want the later timestamp (较晚)", got[0].Content) + } + if got[1].Content != "较早" { + t.Errorf("got[1].Content = %q, want the earlier timestamp (较早)", got[1].Content) + } +} + +// TestStore_List_同一时间戳_插入序 covers the tie case: the host clock is coarse +// enough that three consecutive Adds can share one timestamp, so ordering must +// fall back to rowid rather than an arbitrary row order. +func TestStore_List_同一时间戳_插入序(t *testing.T) { + t.Parallel() + ctx := context.Background() + s := newTestStore(t) + + at := time.Date(2026, 9, 7, 3, 20, 5, 830978900, time.UTC) + for _, content := range []string{"第一条", "第二条", "第三条"} { + item := MemoryItem{PersonaID: "persona.a", Content: content, CreatedAt: at, UpdatedAt: at} + if _, err := s.Add(ctx, item); err != nil { + t.Fatalf("Add(%s): %v", content, err) + } + } + + want := []string{"第三条", "第二条", "第一条"} + for _, tc := range []struct { + name string + list func(context.Context) ([]MemoryItem, error) + }{ + {"List", s.List}, + {"ListByPersona", func(ctx context.Context) ([]MemoryItem, error) { + return s.ListByPersona(ctx, "persona.a") + }}, + } { + got, err := tc.list(ctx) + if err != nil { + t.Fatalf("%s: %v", tc.name, err) + } + if len(got) != len(want) { + t.Fatalf("%s = %d items, want %d", tc.name, len(got), len(want)) + } + for i, w := range want { + if got[i].Content != w { + t.Errorf("%s got[%d].Content = %q, want %q (rowid DESC)", tc.name, i, got[i].Content, w) + } + } + } +} + func TestStore_Delete_正常(t *testing.T) { t.Parallel() ctx := context.Background() diff --git a/internal/runtime/memory/timefmt.go b/internal/runtime/memory/timefmt.go new file mode 100644 index 0000000..fafa0c8 --- /dev/null +++ b/internal/runtime/memory/timefmt.go @@ -0,0 +1,135 @@ +package memory + +import ( + "context" + "database/sql" + "fmt" + "strings" + "time" +) + +// timeLayout is the fixed-width timestamp layout used for every stored +// created_at/updated_at value. Unlike time.RFC3339Nano it never drops trailing +// fractional zeros, so every value is exactly storedTimeLen bytes wide. SQLite +// compares these TEXT columns as plain strings, and equal width is what makes +// that comparison agree with chronological order. UTC values render the zone as +// a single "Z". +const timeLayout = "2006-01-02T15:04:05.000000000Z07:00" + +// storedTimeLen is the byte length of a UTC value formatted with timeLayout: +// "2006-01-02T15:04:05.000000000Z" — the Z07:00 zone collapses to "Z" in UTC. +const storedTimeLen = 30 + +// memoryItemsTable is the table whose timestamps are normalized when the store +// opens. It must match the DDL in store.go. +const memoryItemsTable = "memory_items" + +// formatTime encodes t in UTC with the fixed-width timeLayout. Always writing +// UTC keeps ordering and round-trips stable across processes, and the fixed +// width is what lets ORDER BY created_at order these TEXT columns by time. +func formatTime(t time.Time) string { + return t.UTC().Format(timeLayout) +} + +// parseTime decodes a stored timestamp. time.RFC3339Nano accepts both the +// fixed-width values this package writes and the legacy variable-width ones, so +// the same parser serves reads and the open-time normalization. Blank input +// yields a zero time. +func parseTime(s string) (time.Time, error) { + if strings.TrimSpace(s) == "" { + return time.Time{}, nil + } + t, err := time.Parse(time.RFC3339Nano, s) + if err != nil { + return time.Time{}, fmt.Errorf("memory: parse time %q: %w", s, err) + } + return t, nil +} + +// normalizeMemoryTimestamps rewrites every memory_items timestamp that is not +// storedTimeLen bytes wide into the fixed-width timeLayout. Rows written before +// the layout existed carry variable-width RFC3339Nano text whose string order +// is not its time order; this brings them in line. +// +// It is idempotent: only rows whose column length differs from storedTimeLen are +// selected, so running it again on a normalized table is a no-op. A row whose +// value does not parse is left exactly as it is — one bad legacy value must not +// keep the store from opening. +func normalizeMemoryTimestamps(ctx context.Context, db *sql.DB) error { + if err := normalizeTimeColumn(ctx, db, memoryItemsTable, "created_at"); err != nil { + return err + } + return normalizeTimeColumn(ctx, db, memoryItemsTable, "updated_at") +} + +// normalizeTimeColumn rewrites every table.col value that is not storedTimeLen +// bytes wide. table and col are compile-time constants, never user input, so +// splicing them into the statement is safe; the length comparison and every +// value written are parameterized. All rewrites are collected before any UPDATE +// runs, so no write happens while the SELECT cursor is still open. +func normalizeTimeColumn(ctx context.Context, db *sql.DB, table, col string) error { + fixes, err := collectLegacyTimestamps(ctx, db, table, col) + if err != nil { + return err + } + for _, f := range fixes { + if _, err := db.ExecContext(ctx, + "UPDATE "+f.table+" SET "+f.col+" = ? WHERE id = ?", f.value, f.id); err != nil { + return fmt.Errorf("memory: normalize %s.%s: %w", f.table, f.col, err) + } + } + return nil +} + +// timeFix is one pending timestamp rewrite: which table and column, which row +// (by primary key id), and the fixed-width value to write. +type timeFix struct { + table string + col string + id string + value string +} + +// collectLegacyTimestamps reads the rows of table.col whose value is not +// storedTimeLen bytes wide and returns the parseable ones as pending rewrites. +// Unparseable values are skipped, so normalization can never fail a row: the +// only errors are genuine read failures. +func collectLegacyTimestamps(ctx context.Context, db *sql.DB, table, col string) ([]timeFix, error) { + rows, err := db.QueryContext(ctx, + "SELECT id, "+col+" FROM "+table+" WHERE length("+col+") != ?", storedTimeLen) + if err != nil { + return nil, fmt.Errorf("memory: query legacy %s.%s: %w", table, col, err) + } + defer func() { _ = rows.Close() }() + + fixes := make([]timeFix, 0) + for rows.Next() { + var id, raw string + if err := rows.Scan(&id, &raw); err != nil { + return nil, fmt.Errorf("memory: scan legacy %s.%s: %w", table, col, err) + } + fixed, ok := reformatTime(raw) + if !ok { + continue + } + fixes = append(fixes, timeFix{table: table, col: col, id: id, value: fixed}) + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("memory: iterate legacy %s.%s: %w", table, col, err) + } + return fixes, nil +} + +// reformatTime re-encodes raw with timeLayout. The bool is false when raw is +// already storedTimeLen wide or when it does not parse, in which case the caller +// leaves the stored value untouched. +func reformatTime(raw string) (string, bool) { + if len(raw) == storedTimeLen { + return "", false + } + t, err := parseTime(raw) + if err != nil || t.IsZero() { + return "", false + } + return formatTime(t), true +} diff --git a/internal/runtime/memory/timefmt_test.go b/internal/runtime/memory/timefmt_test.go new file mode 100644 index 0000000..92d1a60 --- /dev/null +++ b/internal/runtime/memory/timefmt_test.go @@ -0,0 +1,148 @@ +package memory + +import ( + "context" + "database/sql" + "testing" + "time" + + "git.yeij.top/AskaEth/Cyrene_v2/internal/db" +) + +// newTestDB opens an isolated in-memory SQLite DB for tests that must inspect +// the raw stored timestamp text. +func newTestDB(t *testing.T) *sql.DB { + t.Helper() + ctx := context.Background() + d, err := db.Open(ctx, ":memory:", testLogger()) + if err != nil { + t.Fatalf("open in-memory db: %v", err) + } + t.Cleanup(func() { _ = d.Close() }) + return d +} + +// insertLegacyItemSQL writes a memory_items row with caller-supplied raw +// timestamps, standing in for a row persisted before the fixed-width layout. +const insertLegacyItemSQL = `INSERT INTO memory_items + (id, persona_id, role_id, project_id, scope, kind, content, tags, confidence, + source_conversation, created_at, updated_at, decay_weight) + VALUES (?, 'persona.a', '', '', 'persona', 'relationship', '旧数据', '[]', 1.0, '', ?, ?, 1.0)` + +func TestFormatTime_定宽_正常(t *testing.T) { + t.Parallel() + tests := []struct { + name string + at time.Time + want string + }{ + {"零小数秒", time.Date(2026, 9, 7, 3, 20, 5, 0, time.UTC), "2026-09-07T03:20:05.000000000Z"}, + {"一位小数秒", time.Date(2026, 9, 7, 3, 20, 5, 830000000, time.UTC), "2026-09-07T03:20:05.830000000Z"}, + {"纳秒", time.Date(2026, 9, 7, 3, 20, 5, 830978900, time.UTC), "2026-09-07T03:20:05.830978900Z"}, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := formatTime(tt.at) + if got != tt.want { + t.Errorf("formatTime = %q, want %q", got, tt.want) + } + if len(got) != storedTimeLen { + t.Errorf("formatTime length = %d, want %d", len(got), storedTimeLen) + } + }) + } +} + +func TestFormatTime_非UTC_正常(t *testing.T) { + t.Parallel() + zone := time.FixedZone("UTC+8", 8*60*60) + // 11:20:05.830+08:00 is 03:20:05.830Z, so the stored text is UTC. + at := time.Date(2026, 9, 7, 11, 20, 5, 830000000, zone) + if got, want := formatTime(at), "2026-09-07T03:20:05.830000000Z"; got != want { + t.Errorf("formatTime = %q, want %q", got, want) + } +} + +// TestNewStore_归一化历史时间戳_正常 writes a legacy variable-width row directly, +// re-opens the store to trigger normalization, and checks both the repaired text +// and its idempotence on a further open. +func TestNewStore_归一化历史时间戳_正常(t *testing.T) { + t.Parallel() + ctx := context.Background() + d := newTestDB(t) + s, err := NewStore(ctx, d, testLogger()) + if err != nil { + t.Fatalf("NewStore: %v", err) + } + + // RFC3339Nano drops the trailing fractional zeros, so this legacy value is + // only 23 bytes wide and loses the text-ordering guarantee. + const legacy = "2026-09-07T03:20:05.83Z" + if _, err := d.ExecContext(ctx, insertLegacyItemSQL, "legacy", legacy, legacy); err != nil { + t.Fatalf("insert legacy row: %v", err) + } + + if _, err := NewStore(ctx, d, testLogger()); err != nil { + t.Fatalf("NewStore (normalize): %v", err) + } + var raw string + if err := d.QueryRowContext(ctx, + `SELECT created_at FROM memory_items WHERE id = ?`, "legacy").Scan(&raw); err != nil { + t.Fatalf("read normalized created_at: %v", err) + } + const want = "2026-09-07T03:20:05.830000000Z" + if raw != want { + t.Errorf("normalized created_at = %q, want %q", raw, want) + } + + got, err := s.Get(ctx, "legacy") + if err != nil { + t.Fatalf("Get: %v", err) + } + wantTime := time.Date(2026, 9, 7, 3, 20, 5, 830000000, time.UTC) + if !got.CreatedAt.Equal(wantTime) || !got.UpdatedAt.Equal(wantTime) { + t.Errorf("timestamps = %v/%v, want %v", got.CreatedAt, got.UpdatedAt, wantTime) + } + + // A further open must be a no-op: the length condition selects nothing once + // the row is fixed width. + if _, err := NewStore(ctx, d, testLogger()); err != nil { + t.Fatalf("NewStore (second pass): %v", err) + } + var again string + if err := d.QueryRowContext(ctx, + `SELECT created_at FROM memory_items WHERE id = ?`, "legacy").Scan(&again); err != nil { + t.Fatalf("read created_at after second open: %v", err) + } + if again != raw { + t.Errorf("second open changed created_at: %q -> %q", raw, again) + } +} + +// TestNewStore_无法解析的时间戳_跳过 pins the "leave it alone" rule: one bad legacy +// value must not keep the store from opening, and must not be rewritten. +func TestNewStore_无法解析的时间戳_跳过(t *testing.T) { + t.Parallel() + ctx := context.Background() + d := newTestDB(t) + if _, err := NewStore(ctx, d, testLogger()); err != nil { + t.Fatalf("NewStore: %v", err) + } + if _, err := d.ExecContext(ctx, insertLegacyItemSQL, "broken", "not-a-time", "not-a-time"); err != nil { + t.Fatalf("insert unparseable row: %v", err) + } + + if _, err := NewStore(ctx, d, testLogger()); err != nil { + t.Fatalf("NewStore with unparseable timestamp: %v", err) + } + var raw string + if err := d.QueryRowContext(ctx, + `SELECT created_at FROM memory_items WHERE id = ?`, "broken").Scan(&raw); err != nil { + t.Fatalf("read created_at: %v", err) + } + if raw != "not-a-time" { + t.Errorf("created_at = %q, want the unparseable value left as is", raw) + } +}