refactor: unified TableContribution contract for strategy-owned tables (closes #129)#136
Merged
Merged
Conversation
8057ccd to
19b0437
Compare
closes #129) "What tables does TypeGraph own?" was split across four uncoordinated surfaces (Drizzle named exports, tables-factory recursion, strategy raw DDL, per-table ensureXTable methods). Adding a new strategy- or backend-owned table without also wiring an ensureXTable + bootstrap probe re-opened the gap #128 closed. Every owned table now flows through one TableContribution shape. Prerequisite for #135 (durable fulltext materialization), itself the prerequisite for #134 (cross-store transaction adoption): this makes the materialization identity/signature exist and be stable; durable persistence + in-memory latch removal remain #135's scope. Breaking (custom FulltextStrategy implementers only): FulltextStrategy.generateDdl(tableName) is replaced by ownedTables(primaryTableName): readonly StrategyTableContribution[]. Strategies declare their tables Drizzle-free (logicalName, owner, resolved tableName, idempotent createDdl for table + supporting indexes, drizzleModel discriminant); the schema factory resolves declarations into authoritative TableContributions. Shipped strategies (tsvectorStrategy, fts5Strategy) and all internal callers are migrated; consumers using only the shipped strategies need no changes. Public API: - New @nicia-ai/typegraph exports: TableContribution, TableContributionSource, StrategyTableContribution, StrategyDrizzleModel, isDrizzleContribution. - Stable, deployment-independent logicalName plus resolved physical tableName as distinct identity vs. drift-signature inputs — the prerequisite #135 needs. Base contributions key logicalName off the stable factory key (nodes, edges, …), never the custom physical name. Internals: - postgresContributions() / sqliteContributions() are the single source of truth for DDL generation, the bootstrap ensure, and drizzle-kit visibility. The Postgres tables.fulltext pgTable is created once by the factory and that exact object is attached to the fulltext contribution — never a second object for the same physical table. generatePostgresDDL / generateSqliteDDL iterate contributions; the table === tables.fulltext identity hack is gone. drizzle-kit visibility is declarative (source.kind). - ensureContribution(logicalName) materializes a single strategy-declared slot via the strategy's ownedTables (throws on an unknown name), running the full idempotent createDdl so a partial state self-heals. It is not a generic materialize-any-table entrypoint; base/core tables come from drizzle-kit or bootstrapTables. ensureRuntimeContributions() materializes only runtimeEnsure (strategy-owned) contributions and is what loadActiveSchemaWithBootstrap calls — the per-boot path asks the strategy directly and never walks/regenerates base-table DDL. ensureFulltextTable retained as a back-compat wrapper. DDL statement ordering changes from "all CREATE TABLE, then all CREATE INDEX, then fulltext" to per-contribution "table then its own indexes". Safe — TypeGraph's tables carry no cross-table foreign keys — but raw migration SQL byte output differs accordingly. Verified: pnpm fix, typecheck, and knip clean; SQLite suite 3093 passed; PostgreSQL suite 594 passed.
19b0437 to
0c14d79
Compare
Merged
This was referenced May 16, 2026
pdlug
added a commit
that referenced
this pull request
May 16, 2026
Comprehensive cleanup of the surface that churned across the three stacked transaction PRs (#136 TableContribution, #138 durable fulltext, #139 cross-store transactions). Net -179 lines, no behavior change. - Cut the speculative `source` abstraction: removed `TableContributionSource`, `source`, `isDrizzleContribution`, `StrategyDrizzleModel`/`drizzleModel`, `resolveStrategyContribution` (zero production consumers). `StrategyTableContribution` is now a `TableContribution` alias. Removed both `no-explicit-any` escapes; table guards now use Drizzle's real brand check. - Deleted the fully-dead `ensureContribution` API across all four layers + `findStrategyContribution` that only served it. - Collapsed the 3-way-duplicated fulltext gate into one shared `gateFulltextMethods`. - Removed the tx-adoption type-safety escapes: new `InternalOperationBackend` lets the factories return their true shape (no more `as unknown as CommonOperationBackend` x3); shared `assertAdoptedDialect` guard eliminates the wrong-dialect footgun and the last `as Any*Database` casts. - Shared `coerceNumericScore` so SQLite enforces the same `score: number` contract as Postgres. - Readability: `withTransaction` @throws doc, manager ensure-path, `materializeOne` shares the state classifier, comment trim. Pending changesets/docs updated to describe the final API shape. Verified: pnpm fix, typecheck, 3102 SQLite + 601 Postgres/integration tests green.
pdlug
added a commit
that referenced
this pull request
May 16, 2026
Comprehensive cleanup of the surface that churned across the three stacked transaction PRs (#136 TableContribution, #138 durable fulltext, #139 cross-store transactions). Net -179 lines, no behavior change. - Cut the speculative `source` abstraction: removed `TableContributionSource`, `source`, `isDrizzleContribution`, `StrategyDrizzleModel`/`drizzleModel`, `resolveStrategyContribution` (zero production consumers). `StrategyTableContribution` is now a `TableContribution` alias. Removed both `no-explicit-any` escapes; table guards now use Drizzle's real brand check. - Deleted the fully-dead `ensureContribution` API across all four layers + `findStrategyContribution` that only served it. - Collapsed the 3-way-duplicated fulltext gate into one shared `gateFulltextMethods`. - Removed the tx-adoption type-safety escapes: new `InternalOperationBackend` lets the factories return their true shape (no more `as unknown as CommonOperationBackend` x3); shared `assertAdoptedDialect` guard eliminates the wrong-dialect footgun and the last `as Any*Database` casts. - Shared `coerceNumericScore` so SQLite enforces the same `score: number` contract as Postgres. - Readability: `withTransaction` @throws doc, manager ensure-path, `materializeOne` shares the state classifier, comment trim. Pending changesets/docs updated to describe the final API shape. Verified: pnpm fix, typecheck, 3102 SQLite + 601 Postgres/integration tests green.
pdlug
added a commit
that referenced
this pull request
May 16, 2026
…141) Comprehensive cleanup of the surface that churned across the three stacked transaction PRs (#136 TableContribution, #138 durable fulltext, #139 cross-store transactions). Net -179 lines, no behavior change. - Cut the speculative `source` abstraction: removed `TableContributionSource`, `source`, `isDrizzleContribution`, `StrategyDrizzleModel`/`drizzleModel`, `resolveStrategyContribution` (zero production consumers). `StrategyTableContribution` is now a `TableContribution` alias. Removed both `no-explicit-any` escapes; table guards now use Drizzle's real brand check. - Deleted the fully-dead `ensureContribution` API across all four layers + `findStrategyContribution` that only served it. - Collapsed the 3-way-duplicated fulltext gate into one shared `gateFulltextMethods`. - Removed the tx-adoption type-safety escapes: new `InternalOperationBackend` lets the factories return their true shape (no more `as unknown as CommonOperationBackend` x3); shared `assertAdoptedDialect` guard eliminates the wrong-dialect footgun and the last `as Any*Database` casts. - Shared `coerceNumericScore` so SQLite enforces the same `score: number` contract as Postgres. - Readability: `withTransaction` @throws doc, manager ensure-path, `materializeOne` shares the state classifier, comment trim.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #129.
Unified
TableContributioncontract for strategy-owned tables. "What tables does TypeGraph own?" was split across four uncoordinated surfaces (Drizzle named exports, tables-factory recursion, strategy raw DDL, per-tableensureXTablemethods); adding a new strategy/backend table without also wiring anensureXTable+ bootstrap probe re-opened the gap #128 closed. Everything now flows through one shape.This is the blocking prerequisite for #135 (durable fulltext materialization), which is in turn the prerequisite for #134 (cross-store transaction adoption). It deliberately stops short of #135's scope: it makes the materialization identity/signature exist and be stable; durable persistence + latch removal is #135.
Breaking change (custom
FulltextStrategyimplementers only)FulltextStrategy.generateDdl(tableName): string[]→ownedTables(primaryTableName): readonly StrategyTableContribution[]. Strategies now declare their tables Drizzle-free (logicalName,owner, resolvedtableName, idempotentcreateDdlfor table + supporting indexes,drizzleModeldiscriminant); the schema factory resolves declarations into authoritativeTableContributions. Shipped strategies (tsvectorStrategy,fts5Strategy) and all internal callers are migrated — consumers using only the shipped strategies need no changes. Versionedminor.What ships
TableContribution,TableContributionSource,StrategyTableContribution,StrategyDrizzleModel,isDrizzleContribution. StablelogicalName+ resolved physicaltableNameare distinct identity vs. drift-signature inputs (the prerequisite Fulltext storage init: durable, enforced materialization instead of an in-memory per-backend latch on the hot path #135 needs).postgresContributions()/sqliteContributions()drive DDL generation, the bootstrap ensure, and drizzle-kit visibility. The Postgrestables.fulltextpgTable is created once by the factory and that exact object is attached to the fulltext contribution — never a duplicate (reference-identity test enforces this). Thetable === tables.fulltexthack is gone; drizzle-kit visibility is now declarative (source.kind).ensureContribution(logicalName)materializes a single strategy-declared contribution (the slotsFulltextStrategy.ownedTablesdeclares —"fulltext"today), running its full idempotentcreateDdl(table + indexes) so partial state self-heals — not probe-and-skip. It is intentionally not a generic "materialize any table" entrypoint: base/core tables come from drizzle-kit orbootstrapTables, and an unknownlogicalNamethrows rather than silently no-ops.loadActiveSchemaWithBootstrapcallsensureRuntimeContributions(), scoped toruntimeEnsure(strategy-owned) contributions only, so startup does not regress into broad DDL/probing.ensureFulltextTableretained as a back-compat wrapper.Two deliberate deviations from the issue text (defended in-code)
ddl.ts, not the factoryas constreturn — baking it intocreatePostgresTableswould create aschema → ddlimport cycle and freeze contributions against the default strategy even when a backend overrides it. Same intent, cleaner seam.@deprecated—@typescript-eslint/no-deprecatedfailed the build on the manager's own legitimate back-compat fallback and the shim's tests. A machine-deprecation on a shim the codebase must still call is wrong.Behavioural note
DDL statement ordering changes from "all CREATE TABLE → all CREATE INDEX → fulltext" to per-contribution "table then its own indexes". Safe — TypeGraph's tables carry no cross-table foreign keys — but raw migration SQL byte output differs accordingly.
Verification
pnpm fix— cleanpnpm typecheck— cleanpnpm test(SQLite) — 3092 passed, 0 failedtests/backends/postgres/+tests/backends/integration/) — 594 passed, 0 failed (incl.postgres-fulltext-bootstrap,postgres-fulltext)New contract test (
tests/table-contribution.test.ts) covers: no duplicate pg table object, custom table names in contribution identity, FTS5 raw-ddl, tsvector drizzle-visible, supporting indexes still emitted,runtimeEnsureclassification, and a custom strategy plugging in via the newownedTablesAPI.