Skip to content

[v3.8.50] refactor(db): add combo repository boundary - #8757

Merged
diegosouzapw merged 3 commits into
cheaper-inference:release/v3.8.50from
xiaoyaner0201:feat/db-combo-repository-slice
Aug 6, 2026
Merged

diegosouzapw merged 3 commits into
cheaper-inference:release/v3.8.50from
xiaoyaner0201:feat/db-combo-repository-slice

Conversation

@xiaoyaner0201

@xiaoyaner0201 xiaoyaner0201 commented Jul 27, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Introduce backend-neutral repository contracts for combo definitions and model-to-combo mappings.
  • Move the existing SQLite behavior behind concrete adapters selected by a single composition root, while keeping the current facade API and side effects intact.
  • Add a reusable conformance harness for CRUD, uniqueness, deterministic ordering/pagination, affected-row behavior, corrupt rows, and atomic related-record cleanup.
  • Preserve the legacy synchronous getCombosCount() API through an explicit compatibility binding in the composition root; all other repository operations remain async internally.
  • This is a behavior-preserving persistence-boundary slice: no external database driver, database configuration, schema/migration, startup, or user-facing behavior change.

Related Issues

Validation

Run only the focused loop for what you changed — the full unit suite, Vitest, the
60% coverage gate, and the production build all run in CI on this PR (#8329):

  • Focused tests for the refreshed change: 59/59 passed across seven independently launched conformance, facade CRUD, model-mapping, cache/backup, and quota-combo suites on Node 24
  • npm run lint — the changed files pass ESLint and Prettier; the repository-level static gates listed below also pass
  • Production-code changes include a new or updated automated test in this PR
  • SonarQube PR analysis is green or any remaining issues are explicitly documented below — pending PR CI

Additional local gates passed:

  • npm run typecheck:core
  • changed-file ESLint and prettier --check
  • npm run check:cycles
  • npm run check:db-rules
  • file-size, complexity, cognitive-complexity, and complexity-ratchet checks
  • mutation coverage and test discovery/runner/policy checks
  • git diff --check upstream/release/v3.8.50...HEAD
  • SQLite coupling audit on the exact staged tree: no new application-layer boundary violations or outside-boundary writes

Tests Added Or Updated

  • Added tests/helpers/persistence/comboRepositoryConformance.ts.
  • Added tests/unit/db/repositories/sqliteComboRepositories.test.ts.
  • Updated tests/unit/combo-cache-invalidation.test.ts to characterize cache invalidation, backups, affected-row behavior, reorder behavior, and malformed-row behavior across the facade/repository boundary.

Coverage Notes

  • The conformance harness exercises both SQLite repositories through the backend-neutral contracts.
  • Existing facade CRUD, pagination, routing, cache invalidation, backup, and quota-combo tests exercise the migrated call path end to end.
  • The mutation-coverage gate passes; no touched production file is left without automated coverage.

Reviewer Notes

  • Architecture review gate: the maintainer re-homed this PR to release/v3.8.50 and requested one-to-one review under Hard Rule build(deps): bump actions/github-script from 7 to 8 #2. The branch has now been refreshed without force-push; no external-backend work will be stacked upstream until this boundary is accepted or reshaped.
  • Refreshed base: release/v3.8.50@7163081f5ed2a2104e85c31bfd1588033d43c580; refreshed head: abe7ea7026516f4751ff9338534b4b140ec7877c; validated tree: e3688ca5451aa2f2afbe73630c74cc8c29c4ee29.
  • No new dependency, environment variable, external database, migration, credential handling, or deployment claim is introduced.
  • active_combo runtime state remains outside this bounded slice. The facade retains its existing direct SQLite write for that separate state; combo definition and model-mapping consumers no longer import concrete repository adapters.
  • The SQLite coupling audit therefore shows no new application-layer violations and unchanged total prepare() occurrences; this PR establishes the boundary rather than claiming repository-wide coupling reduction.
  • An independent adversarial review found and drove fixes for synchronous API compatibility, reorder side-effect parity with corrupt rows, composition-root ownership of the sync compatibility seam, and conformance-hook awaiting/capability semantics.

Hard Rule #2 / localDb.ts review map

This PR keeps src/lib/localDb.ts as a re-export-only compatibility layer:

  • src/lib/localDb.ts is byte-for-byte unchanged from the pinned base to this PR head (the Git blob is 70a9dee74f26d8b1160afa86faedbb8047ac9701 at both the refreshed base and head).

  • Its existing re-exports for db/combos and db/modelComboMappings remain unchanged; no repository construction, SQL, or business logic is added there.

  • Dependency direction is explicit:

    existing consumers
      -> db/combos.ts + db/modelComboMappings.ts compatibility facades
      -> repositories/routingConfigRepositories.ts (single composition root)
      -> sqliteComboRepository.ts + sqliteModelComboMappingRepository.ts
      -> db/core.ts getDbInstance()
    
  • The portable contracts live in src/domain/persistence/comboRepositories.ts and contain no SQLite dependency.

  • Concrete SQLite adapters are imported only by routingConfigRepositories.ts; the facades and application consumers do not import them directly.

  • Raw SQL for the selected combo/mapping slice lives in the SQLite adapters. The pre-existing setActiveCombo() write remains in db/combos.ts because active runtime selection is explicitly outside this bounded slice; no part of it is moved into localDb.ts.

  • npm run check:db-rules passes on refreshed head abe7ea7026516f4751ff9338534b4b140ec7877c:
    OK (110 db modules, 73 re-exported, 37 intentionally internal; Rule #2).

The nine-file change is one bounded vertical slice: contract, SQLite implementation, composition binding, compatibility facades, and executable conformance evidence. I do not see a smaller independently buildable cherry-pick subset that would preserve behavior and retain the tests. If the preferred review shape is different, I can split or reshape it after the one-to-one boundary review rather than guessing the cut line.


base-red inherited: #9298 -- Conflict resolved in src/lib/db/combos.ts (kept HEAD repository-based getCombos() over base inline SQLite). Pre-existing migration collision (version 135 and 136 duplicates) on release/v3.8.50 prevents DB-dependent tests from passing. PR code confirmed structurally correct.

@diegosouzapw

Copy link
Copy Markdown
Collaborator

⏸️ Ejetada do merge-train v3.8.49 — triagem de cherrypick pendente

Esta PR foi ejetada do merge-train local (30→24 PRs) porque sua pegada no diff excede o orçamento seguro de validação em lote (>700 linhas ou nova fronteira arquitetural). Não é rejeição — é adiamento com motivo rastreável.

Motivo da ejeção

PR Título Files Linhas Risco em trem
#8760 feat: Vendo platform (novo módulo) 24 +3046 Novo módulo 3046+ linhas — precisa revisão arquitetural antes de entrar no release
#8713 fix(github): Copilot targetFormat override 581 +36652/-29407 Mass migration 36k+ linhas — provavelmente regen de lockfile + tree-wide diff, alto risco de regressão silenciosa
#8673 fix(security): tar ^7.5.22 (GHSA-w8wr-v893-vjvp) 578 +36542/-29411 Dependabot mass bump — GHSA real mas precisa validação isolada + tests de regressão de extract/archive
#8757 refactor(db): combo repository boundary (DRAFT) 9 +1114/-553 Draft + fronteira em DB core — refactor arquitetural, exige revisão de compat com localDb.ts (Hard Rule #2)
#8752 feat(memory): MemoryBackend provider pattern 17 +1347 Novo padrão de provider com HTTP connector genérico — abre nova superfície, exige revisão do contrato
#8718 fix(test): revive orphaned vitest tests 22 +733/-772 Suite-wide em vitest — risco de timing-flakes mascarando regressões (medido em #6218 follow-ups)

Como destravar (sugestão do operador)

O operador pediu que toda PR ejetada seja revisada para possível cherrypick. Providências sugeridas por categoria:

  1. PRs com subconjunto cherrypickável (feat: add Vendo platform - African commerce MVP foundation #8760, [v3.8.50] feat(memory): MemoryBackend provider pattern with generic HTTP connector #8752, [v3.8.50] fix(test): revive orphaned vitest tests and fix CI routing #8718): abrir gh pr diff <N> e identificar arquivos pequenos, isolados, sem dependência cross-module. Considerar:

    git remote add author-<login> <fork-url> 2>/dev/null
    git fetch author-<login> <branch>
    git checkout -b cherry/<N>-<slug> release/v3.8.49
    git cherry-pick <commit-sha-do-subconjunto>

    Abrir nova PR com o subconjunto (mantendo Co-authored-by do autor original).

  2. PRs com regen/lockfile ([v3.8.50] fix(github): honor per-model targetFormat override for Copilot custom models #8713, [v3.8.50] fix(security): override tar to ^7.5.22 (GHSA-w8wr-v893-vjvp) — unblocks the Lint job on every PR #8673): provavelmente só o package.json + package-lock.json + o patch de código mínimo. Rodar npm ls tar no tip local para confirmar a versão segura antes de cherrypick.

  3. PRs com refactor arquitetural ([v3.8.50] refactor(db): add combo repository boundary #8757): exige review 1-a-1 com o operador — a fronteira db/ precisa ser validada contra Hard Rule build(deps): bump actions/github-script from 7 to 8 #2 (nunca lógica em localDb.ts).

Próximos passos

  • Comentário fixado para rastreabilidade.
  • Labels aplicadas: hold-vps, needs-rebase, merge-train-deferred.
  • Estas 6 NÃO bloqueiam o trem das outras 24 (que vai entrar em modo --fast na .113).

@xiaoyaner0201

Copy link
Copy Markdown
Contributor Author

Thanks for the explicit triage and for keeping the PR traceable. I agree that this boundary should not enter through the batch merge train without the one-to-one architecture review.

I audited the current exact head specifically against Hard Rule #2:

  • src/lib/localDb.ts is byte-for-byte unchanged from the pinned base to this PR head (same blob 889792e825b01f2c77015ccadd819f5c78285ca3).
  • No logic, SQL, repository construction, or new import was added to that re-export layer.
  • Existing consumers continue through the db/combos.ts and db/modelComboMappings.ts compatibility facades.
  • Those facades depend on the backend-neutral contracts and the single repositories/routingConfigRepositories.ts composition root; only that composition root imports the concrete SQLite adapters.
  • The SQL for this slice is confined to the concrete SQLite adapter files. The existing setActiveCombo() direct SQLite write remains outside the slice and was not moved into localDb.ts.
  • npm run check:db-rules passes on exact head d2a5e1c71af621f31e39227904259c166f47bba1.

I added the exact dependency map and review anchors to the PR description so the one-to-one review does not need to infer the boundary from the nine-file diff.

For cherrypick triage, I do not currently see a smaller independently buildable subset: the contract, composition binding, SQLite adapters, compatibility facades, and conformance tests form one behavior-preserving vertical slice. I am happy to split or reshape it if the preferred cut line differs after review.

One sequencing question to avoid invalidating the frozen-tree evidence twice: should I rebase this Draft onto the current v3.8.50 release target now, or keep this exact head stable until the Hard Rule #2 boundary review is complete and then rebase once against the confirmed target?

@diegosouzapw
diegosouzapw changed the base branch from release/v3.8.49 to release/v3.8.50 July 28, 2026 18:41
@diegosouzapw

Copy link
Copy Markdown
Collaborator

Re-homed to release/v3.8.50: v3.8.49 entered its release freeze, so the branch now belongs to the release captain and development continues on the next cycle. Nothing is wrong with this PR — it just needed a live base. No action needed from you; CI will re-run against the new base.

Merge the current release/v3.8.50 base without changing the validated repository slice.
@xiaoyaner0201
xiaoyaner0201 marked this pull request as ready for review August 3, 2026 22:04
@xiaoyaner0201

Copy link
Copy Markdown
Contributor Author

The branch is now refreshed onto the current release/v3.8.50 tip without rewriting the public branch history.

Fresh validation on the resulting tree:

  • 59/59 focused repository/facade/cache/quota tests pass on Node 24
  • typecheck:core, cycle checks, and DB boundary rules pass
  • the SQLite coupling audit reports no new application-layer boundary violations
  • the PR remains the same nine-file vertical slice; no external driver, configuration, schema, migration, or runtime behavior change was added

This is ready for the one-to-one Hard Rule #2 architecture review requested above. To make the review decision concrete, could you confirm one of these directions?

  1. Keep this contract + SQLite adapter + composition root + compatibility facade + conformance tests as one reviewable slice; or
  2. Use a different cut line, in which case I will reshape this PR before proposing any follow-up persistence work.

I will not expand the upstream scope into an external database backend until this boundary decision is made.

…flict resolution)

Resolved conflict in src/lib/db/combos.ts: kept HEAD (repository-based
getCombos) over base's inline SQLite implementation, which the PR
refactors behind the ComboRepository interface.
@diegosouzapw

Copy link
Copy Markdown
Collaborator

Merge conflict resolved (2026-08-06)

Conflict in src/lib/db/combos.ts:

  • HEAD (PR): repository-based getCombos() via repository.list(limit, offset)
  • Base (origin/release/v3.8.50): old inline SQLite implementation with helper functions (asRecord, parseComboRow, etc.)

Resolution: kept HEAD (repository-based approach), since the PR's entire purpose is the combo repository boundary refactor.

Note: This PR inherits base-red issue #9298 (migration collision at versions 135 and 136). All 6 PR test files load correctly but cannot pass due to the base issue. Typecheck running.

@diegosouzapw
diegosouzapw marked this pull request as ready for review August 6, 2026 22:11
@diegosouzapw
diegosouzapw merged commit 5f47118 into cheaper-inference:release/v3.8.50 Aug 6, 2026
20 of 22 checks passed
jonlwheat2-gif added a commit to jonlwheat2-gif/OmniRoute that referenced this pull request Aug 22, 2026
…nce#8075)

Add a dialect-agnostic DatabaseBackend abstraction that decouples domain
modules from the storage engine. SQLite remains the default (zero-config);
PostgreSQL and MySQL are opt-in for cluster/centralized deployments via
DATABASE_DRIVER env var.

Scope: PR4 equivalent (backend operational skeleton) per the staged plan
in issue cheaper-inference#8075. Domain repository contracts (PR3) already exist upstream
from cheaper-inference#8757. Settings/cleanup/API route migrations are deferred to later
slices.

Core:
- DatabaseBackend interface with async query/execute/transaction/healthCheck
- SqliteBackend wrapping existing better-sqlite3 (zero behavioral change)
- PostgresBackend with pg connection pooling and SQL translation
- MysqlBackend with mysql2 connection pooling and SQL translation
- SqlDialect compiler: translates SQLite-flavored SQL (positional ? params,
  INSERT OR REPLACE, PRAGMA table_info, AUTOINCREMENT, BLOB) to Postgres/MySQL
- Schema registry mapping table PKs for dialect-aware upsert translation
- Lazy singleton factory (DATABASE_DRIVER=sqlite|postgres|mysql, default sqlite)
- Config via DATABASE_URL or individual DATABASE_HOST/PORT/NAME/USER/PASSWORD
- Automatic fallback to SQLite if external backend connection fails

PR4 additions:
- Classified persistence errors (PersistenceError with retryable/non-retryable codes)
- Migration lock contract (process-local, seam for future distributed lock)

Migration:
- Dialect-aware migration runner for external backends (reads same .sql files)
- Offline SQLite → PostgreSQL/MySQL export tool (exportSqliteToDialect)
- Migrations tracked in _omniroute_migrations table

Tests (90 passing):
- SqliteBackend: query, queryOne, execute, transactions, rollback, health
- SqlDialect: placeholder conversion, INSERT OR REPLACE/IGNORE translation,
  PRAGMA table_info, DDL type mapping (AUTOINCREMENT→SERIAL, BLOB→BYTEA)
- PersistenceError: classification of 14 error patterns (transient, constraint, auth, migration)
- Backend conformance: timestamps, JSON round-trip, affected rows, NULL handling,
  transaction atomicity, backend identity
- Combo repository conformance: CRUD, pagination, ordering, uniqueness, reorder,
  delete isolation, case-insensitive lookup

Env vars documented in .env.example. pg + mysql2 added as optional deps.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deferred-v3.8.50 Adiada para o ciclo v3.8.50 (validacao VPS, refactor, ou escopo grande) hold-vps PR verde, merge aguardando validação live (release-drain) merge-train-deferred PR ejetada do merge-train — triagem separada needs-rebase PR ejetada do merge-train — triagem separada

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants