Skip to content

Simplify SQL access away from giant store modules #1600

Description

@sentry-junior

SQL access is growing through large store modules and object facades. Prefer feature-local Drizzle against tables. Keep helpers only for real invariants or shared non-trivial mapping.

Intent

Agreed direction from design review:

  • Drizzle + table schema is the data access layer.
  • Default: query tables in the owning feature or plugin module.
  • Extract a function only when a write rule is load-bearing, or the same non-trivial mapping repeats.
  • Do not invent CRUD facades (get/list/create/save/delete) that only wrap one query.
  • Keep object *Store factories only for a real edge (today: conversation DI). Do not grow new ones for normal features.
  • No global repositories/, DAO framework, or new persistence vocabulary.
  • src/db/ stays connection + schema. Feature/plugin folders own ops next to their tools and runtime code.
  • Split large files by domain concern only after the facade is gone, and only when the file is still large.

This matches existing healthy code (event-tasks, workspaces, github outcomes, many api/ readers) and existing policy (policies/interface-design.md, policies/correctness-complexity.md, chat README ownership by feature).

Audit

store.ts inventory (current main)

Lines Backend Path
2008 SQL + legacy state packages/junior/src/chat/scheduled-tasks/store.ts
1617 SQL packages/junior-memory/src/store.ts
898 SQL class/ConversationStore packages/junior/src/chat/conversations/sql/store.ts
579 state packages/junior/src/chat/agent-dispatch/store.ts
572 state packages/junior/src/chat/resource-events/store.ts
558 SQL packages/junior/src/chat/attachments/store.ts
556 mailbox/lease (not SQL facade) packages/junior/src/chat/task-execution/store.ts
439 SQL packages/junior/src/chat/agent-invocations/store.ts
360 SQL plain functions packages/junior/src/chat/workspaces/store.ts
304 SQL packages/junior/src/chat/artifacts/store.ts
299 SQL plain functions packages/junior-github/src/pull-request-outcomes/store.ts
205 SQL plain functions packages/junior/src/chat/event-tasks/store.ts
131 SQL packages/junior/src/personal-tokens/store.ts
128 SQL plain functions packages/junior-github/src/issue-outcomes/store.ts
145 interface only packages/junior/src/chat/conversations/store.ts

File-length exceptions already call out the two primary outliers and say “split by storage concern”:

  • scripts/file-length-exceptions.mjsscheduled-tasks/store.ts, junior-memory/src/store.ts

Object facades vs plain functions

Object/create*Store edges still present:

  • ConversationStore / createSqlStore / event + message-search store factories — real DI across task-execution
  • SchedulerStore / createSchedulerSqlStore / createSchedulerStore — dual backend leftover
  • MemoryStore / createMemoryStore — convenience bag over closed-over context, not a second backend
  • UserTokenStore — credential edge, not SQL table CRUD

Plain-function SQL modules already in good shape:

  • event-tasks/store.ts
  • workspaces/store.ts
  • github pull-request-outcomes/store.ts, issue-outcomes/store.ts
  • many direct table readers outside stores (api/people/*, chat/tasks/read.ts, execution-stats.ts, etc.; ~230 direct schema references outside store/sql modules)

Scheduler dual backend

  • createSchedulerStore(state) appears production-dead; only packages/junior/tests/unit/scheduler-state-index.test.ts calls it.
  • Production/eval paths use createSchedulerSqlStore(getDb()) from heartbeat, tools, tasks read, evals.
  • File still contains both state and SQL implementations plus shared claim/run logic (~2k lines).

Why thin helpers keep appearing

Several tables store a JSON document plus a few indexed columns:

  • scheduler: record jsonb + title/status/nextRunAtMs (db/schema/scheduled-tasks.ts)
  • event tasks: task jsonb + title (db/schema/event-tasks.ts)

That encourages getX wrappers that only do select … → parseRow. Those wrappers are not free architecture; keep them only when many callers share the same parse/filter rule.

Out of scope for this cleanup

  • Redis/state stores that are not SQL facades (agent-dispatch, resource-events, mailbox/lease task-execution/store.ts) unless a later pass wants rename-only consistency
  • Broad schema normalization of all jsonb document columns (helpful later; not required to delete facades)
  • Rewriting conversation DI (ConversationStore) just to avoid the word “store”

Pointed suggestions

P0 — scheduled tasks

  1. Delete or fully isolate the PluginState backend (createSchedulerStore / operational state store) if still test-only.
  2. Remove SchedulerStore as the default call shape. Heartbeat/tools/read should not do createSchedulerSqlStore(db).getTask(...).
  3. Inline trivial task/run reads and writes at call sites with Drizzle against juniorSchedulerTasks / juniorSchedulerRuns.
  4. Keep one named function for the real multi-step rule: due-run claim under lock (today claimDueRun). Same for any terminal run transition that must stay atomic.
  5. Share parseSqlTaskRow / title+record merge only if multiple modules need the exact same decode.
  6. Drop the file-length exception when the god file is gone.

P1 — memory plugin

  1. Stop routing tools/recall/process-session through createMemoryStore(...).method() as the main API.
  2. Keep extracted functions only where domain rules live:
    • create path: idempotency, dedupe, preference supersession, embedding write
    • search/recall path: hybrid retrieval + ranking gates
  3. Simple list/get/archive paths can be plain queries in the caller if they stay simple.
  4. Leave plugin schema ownership in junior-memory/src/db/schema.ts; do not move memory SQL into core src/db/.
  5. Split the 1.6k file by those domain concerns after the object bag is gone; remove the exception entry.

P2 — thin SQL modules / naming

  1. Revisit one-liner getters like getEventTask / getWorkspace only if a caller needs a different projection; do not mass-delete working plain functions just to inline everywhere.
  2. Prefer feature filenames that name the domain (tasks.ts, claim.ts) over mechanical store.ts when touching a file anyway (policies/interface-design.md).
  3. Do not add an sql/ subdirectory unless a feature accumulates many SQL modules the way conversations already has.

P3 — conversations SQL class

  1. Keep ConversationStore while task-execution DI needs it.
  2. Continue shrinking conversations/sql/store.ts by moving distinct concerns into existing sibling modules (bindings, participants, history, etc.).
  3. Do not use conversations as the template for every feature.

Guardrails while cleaning

  • Hard cutover for internal APIs; search every consumer (createSchedulerSqlStore, createMemoryStore, SchedulerStore, MemoryStore).
  • Prove behavior at existing integration/component edges; do not add one unit test per former store method.
  • Avoid new abstraction names (“repository”, “transaction script”, “unit of work”).
  • Prefer deleting code over relocating the same CRUD surface into more files.

Suggested sequence

  1. Scheduler: remove dead state backend + SchedulerStore object API.
  2. Scheduler: inline trivial SQL; keep claim/atomic run transitions as named functions.
  3. Memory: replace createMemoryStore bag with direct ops for create/search; inline simple paths.
  4. Opportunistic renames / thin-wrapper cleanup in event-tasks, workspaces, github outcomes only when already touching those files.
  5. Optional later: reduce jsonb-document reliance so fewer parse helpers are needed.

Done when

  • No production feature requires createXStore(db) for ordinary SQL CRUD.
  • scheduled-tasks/store.ts and junior-memory/src/store.ts are gone or under the 1,000-line limit without exceptions for “storage concern” bags.
  • New SQL code defaults to feature-local Drizzle; helpers exist only for invariants or shared mapping.
  • Conversation DI remains explicit and unbroken.

Requested by David Cramer.

--

View Junior Session [Sentry]

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions