Skip to content

refactor(eventstore): QueryBySession, TurnStats, scanTurns DRY across SQLite/PG backends #518

@hrygo

Description

@hrygo

Background

internal/eventstore 负责会话事件持久化和 delta 聚合,8 文件 ~3,245 行。模块 SOLID 合规良好(清晰接口隔离、DI、文件级 SRP)。核心问题是 3 个大型方法体在 SQLiteStore 和 pgStore 之间复制粘贴,仅方言特定 NULL 类型有差异。

Scope: SOLID, DRY, coupling — cycle 158 (模块分析通过 1)
Key files: store.go, pg_store.go


Finding Summary

Category Critical High Medium Low
DRY 0 0 3 0
合计 0 0 3 0

Findings

DRY

querybysession-duplication-across-backends

Severity: Medium | Confidence: High | ROI: Medium
Location: store.go:328-393, pg_store.go:80-145

Problem: QueryBySession 在 SQLiteStore 和 pgStore 之间复制粘贴。65 行方法体(limit 钳制、cursor 分发、额外行获取、反转逻辑、页面组装)完全相同,仅 query-map 访问器和 db 字段名不同。未来分页逻辑变更(如添加 cursor 方向或修改 has_older 算法)必须在两个文件中重复修改。

Current Pattern:

// store.go:328 and pg_store.go:80 — identical 65 lines
func (s *SQLiteStore) QueryBySession(...) (*EventPage, error) {
    if limit <= 0 { limit = 200 }
    if limit > 1000 { limit = 1000 }
    fetchLimit := limit + 1
    switch dir {
    case CursorAfter:  rows, err = s.db.QueryContext(ctx, queries["query_after"], ...)
    case CursorBefore: rows, err = s.db.QueryContext(ctx, queries["query_before"], ...)
    default:           rows, err = s.db.QueryContext(ctx, queries["query_latest"], ...)
    }
    // ... identical reverse + HasOlder logic
}

Proposed Fix:

type queryFn func(ctx context.Context, key string, args ...any) (*sql.Rows, error)

func queryBySession(ctx context.Context, qfn queryFn, sessionID string, cursor int64, dir CursorDirection, limit int) (*EventPage, error) {
    if limit <= 0 { limit = 200 }
    if limit > 1000 { limit = 1000 }
    // ... shared logic using qfn(ctx, key, args...)
}

func (s *SQLiteStore) QueryBySession(...) (*EventPage, error) {
    qfn := func(ctx context.Context, key string, args ...any) (*sql.Rows, error) {
        return s.db.QueryContext(ctx, queries[key], args...)
    }
    return queryBySession(ctx, qfn, sessionID, cursor, dir, limit)
}

Estimated Impact: 消除 ~65 行重复,分页逻辑统一维护

Acceptance Criteria:

  • queryBySession 共享函数处理 limit 钳制、cursor 路由、页面组装
  • SQLiteStore 和 pgStore 各自仅提供 queryFn 适配器
  • make test 通过,QueryBySession 测试覆盖两个后端

queryturnstats-scan-loop-duplication

Severity: Medium | Confidence: High | ROI: Medium
Location: store.go:492-523, pg_store.go:273-304

Problem: QueryTurnStats 的行扫描和聚合循环(~30 行)在 SQLiteStore 和 pgStore 之间重复。仅差异:(1) success 列使用 NullInt64 vs NullBool (2) 包级 slog vs 注入的 s.log。8 个计数器的聚合数学容易在后端间去同步。

Current Pattern:

// store.go:492 and pg_store.go:273
stats := &TurnStats{SessionID: sessionID, Generation: gen}
for rows.Next() {
    var ts TurnStatItem
    var success sql.NullInt64  // vs sql.NullBool in PG
    rows.Scan(&ts.ID, ..., &success, ...)
    ts.Success = success.Valid && success.Int64 == 1  // vs success.Bool
    stats.TotalTurns++
    stats.TotalDurMs += ts.DurationMs
    stats.TotalCostUSD += ts.CostUSD
    // ... 6 more identical accumulation lines
}

Proposed Fix:

func aggregateTurnStats(rows *sql.Rows, stats *TurnStats, scanFn func(rows *sql.Rows, ts *TurnStatItem) error, log *slog.Logger) error {
    for rows.Next() {
        var ts TurnStatItem
        if err := scanFn(rows, &ts); err != nil { log.Warn(...); continue }
        stats.TotalTurns++
        stats.TotalDurMs += ts.DurationMs
        // ... shared accumulation ...
    }
    if stats.TotalTurns == 0 { return ErrNotFound }
    return rows.Err()
}

Estimated Impact: 消除 ~30 行重复,聚合逻辑统一维护

Acceptance Criteria:

  • aggregateTurnStats 共享函数处理聚合逻辑
  • SQLite 和 PG 各提供 success 列的 scanFn 适配器
  • make test 通过

scanturns-vs-scanturnspg-near-duplicate

Severity: Medium | Confidence: High | ROI: Medium
Location: store.go:569-595, pg_store.go:335-360

Problem: scanTurns (SQLite) 和 scanTurnsPG (PostgreSQL) 95% 相同。26 行中仅 3 行不同:success 列的 NULL 类型(NullInt64 vs NullBool)和 bool 指针赋值方式。21 列 Scan 调用和 tools_json 反序列化是复制粘贴的。

Current Pattern:

// store.go:569 scanTurns
func scanTurns(rows *sql.Rows) ([]*TurnRecord, error) {
    for rows.Next() {
        var success sql.NullInt64    // SQLite: INTEGER
        rows.Scan(&r.ID, &r.SessionID, ..., &success, ..., &toolsJSON, ...)
        if success.Valid {
            s := success.Int64 == 1
            r.Success = &s
        }
    }
}
// pg_store.go:335 scanTurnsPG — same but sql.NullBool + success.Bool

Proposed Fix:

func scanTurns(rows *sql.Rows, scanSuccess func(rows *sql.Rows, r *TurnRecord, cols []any) error) ([]*TurnRecord, error) {
    // shared 21-column scan + tools_json unmarshal
    // scanSuccess handles dialect-specific success column
}

Estimated Impact: 消除 ~23 行重复,21 列 Scan 调用统一维护

Acceptance Criteria:

  • 单一 scanTurns 函数处理共享列扫描,方言适配器处理 success 列
  • schema 列变更只需修改一处
  • make test 通过

Implementation Priority

Finding Priority Effort Risk Impact
querybysession-duplication P1 Medium Low ~65 行减少,分页逻辑统一
scanturns-vs-scanturnspg P1 Small Low ~23 行减少,21 列 Scan 统一
queryturnstats-scan-loop P2 Small Low ~30 行减少,聚合逻辑统一

Recommended starting point: scanTurns 统一(P1/Small/Low,最简单的提取)然后 QueryBySession


Out of Scope

  • SQLiteStore 用 *sql.DB 而 pgStore 用 *dbutil.DB:有意设计,SQLite 需要 WriteMu
  • store.go 用包级 slog vs pg_store.go 用注入 logger:单处外观不一致
  • captureRequest sum-type 用指针字段:Go 惯用法,3 变体内部类型无需密封接口

Verification

  • make test 通过,无回归
  • make lint 不产生新警告
  • 分页和聚合逻辑修改只需编辑一处

Metadata

Metadata

Assignees

No one assigned

    Labels

    P3Medium: tech debt, refactoring, improvementsarchitectureDomain: design patterns, coupling, separation of concernsarea/sessionScope: session manager, state machine, store, poolrefactorRefactor: DRY, SOLID, code quality improvements

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions