Skip to content

Drop the document index - #126

Merged
davidmckayv merged 1 commit into
mainfrom
chore/drop-the-document-index
Aug 22, 2026
Merged

Drop the document index#126
davidmckayv merged 1 commit into
mainfrom
chore/drop-the-document-index

Conversation

@davidmckayv

Copy link
Copy Markdown
Contributor

What this changes

Finishes what #118 started and #124 half-did. documents, chunks and document_acls go, along with the acl_effect enum they were the only user of and the vector extension that existed for the embedding column.

#124 stopped short on purpose, because server/src/connectors/sync-persistence.ts still imported the three tables and dropping them would have failed typecheck there. #97 removed that file and the worker's connector runner. With both merged, the three tables are declared in server/src/db/schema/core.ts and referenced by nothing else in the tree, so this is the schema-only commit #118 described.

The migration destroys data

server/drizzle/0010_drop_the_document_index.sql says so in its first line rather than leaving it to be found. Every row goes, embeddings included, and no later migration brings them back. There is nothing to migrate them to: the replacement is not another table, it is asking the vendor at the moment of the question, so there is no shape for this content to move into.

Two deliberate choices:

  • Named in dependency order, no CASCADE. chunks, then document_acls, then documents. Ours are the only rows that depend on ours, so CASCADE buys nothing and could take a fork's own objects without saying what it took. Without it, such a deployment gets a failed migration instead.
  • DROP EXTENSION IF EXISTS "vector" is RESTRICT, the default. Same reason. I drove this: a database with a fork_embeddings (v vector(3)) table of its own fails the migration, and because the whole thing is one transaction the three tables stay too. All or nothing, not a half-dropped database. Postgres names the dependent column (cannot drop extension vector because other objects depend on it … column v of table fork_embeddings), though drizzle-kit swallows it and prints nothing, which the comment warns about.

The pgvector image stays in compose and CI. It has to: 0000_schema.sql still runs CREATE EXTENSION IF NOT EXISTS vector, so a database built from scratch needs the extension available even though 0010 drops it again.

The test now guards the direction instead of describing the tables

schema.test.ts had a test asserting the shape of all three. Deleting it would have left nothing. It is replaced by one that fails if a document index comes back:

  • It reads every table the schema exports and asserts none is named documents, chunks or document_acls, so a reintroduction under a different name is still caught by the second half.
  • It checks column types, not names, so an embedding smuggled onto a table called something else is caught too.

Both halves fail independently. I checked by adding each back: a documents table fails the name assertion, and a notebook_pages table with a vector column fails the type assertion with neither being named in the test.

Docs

docs/architecture.md:22 and README.md:221 both listed knowledge records among what this database holds. That stops being true in this commit, so it changes in this commit.

Nothing in the README beyond that row: it is a build document.

No changelog entry here. The Unreleased entry is being written elsewhere and I did not want to collide with it.

Where it runs

No new state outliving a request, nothing different on a second replica, nothing serialised, nothing fanned out to a browser, no new listener, port or schedule. One migration and the schema declarations it follows from.

Boundary and audit

N/A. No gateway, policy or audit path touched. Worth saying explicitly that removing document_acls removes no enforcement: that table was our own permission model over a copy of somebody's corpus, and the copy is what is going. Permission now lives where it always should have, at the vendor, checked as the person asking.

Proof

Driven against a real Postgres, not typecheck alone.

Fresh database, 00000010:

[✓] migrations applied successfully!
document tables left: 0
vector extension: 0
acl_effect type: 0

Upgrade path with real rows. Migrated a database to main (0009), inserted a connector instance, a document, a chunk with a 1536-dimension embedding and an ACL row, then applied 0010:

before: documents=1 chunks=1 acls=1 connectors=1
after:  document tables left: 0
        vector extension: 0
        acl_effect type: 0
        connector_instances survived: 1
        users table survived: 1

The rows are gone, which is the point, and nothing adjacent went with them.

Fork case, described above: migration fails, exits 1, three tables and the extension all still present, fork's column untouched.

Migration chain:

$ bunx drizzle-kit check --config=drizzle.config.ts
Everything's fine 🐶🔥

$ bunx drizzle-kit generate --config=drizzle.config.ts --name=ci_drift_probe
No schema changes, nothing to migrate 😴

Suite, against a database migrated to 0010:

result
bun run test 1138 pass, 5 skip, 0 fail, 1143 across 102 files
bun run typecheck clean, all four packages
bun run format:check clean
bunx biome lint . 1 info, identical to main
bun run build clean

Left alone, deliberately

Following #124's lead of flagging rather than sweeping:

All three are worth a change of their own rather than scope creep on this one.

`documents`, `chunks` and `document_acls` have had no reader or writer
since the document connector and the worker's sync persistence were
removed. This drops them, the `acl_effect` enum they were the only user
of, and the `vector` extension that existed for the embedding column.

The migration destroys data and cannot be rolled back, and says so at
the top rather than leaving it to be discovered. It names the tables in
dependency order and does not use CASCADE, so a fork that hung
something off them gets a failed migration that changes nothing instead
of a silent removal. Dropping the extension is RESTRICT for the same
reason.

The architecture and README service tables said this database holds
knowledge records. That stops being true here, so they change here.

The schema test that described the three tables is replaced by one that
fails if a document index comes back: it reads every table the schema
exports rather than named ones, so a rename does not get past it, and
checks column types rather than column names, so an embedding on a
table called something else does not either.
@davidmckayv
davidmckayv merged commit a0f149a into main Aug 22, 2026
6 checks passed
@davidmckayv
davidmckayv deleted the chore/drop-the-document-index branch August 22, 2026 00:22
davidmckayv added a commit that referenced this pull request Aug 22, 2026
#126 and #127 dropped the document index and the old connector tables, so the
changelog line that said the index was "read by nothing" now understates it: the
tables are gone. Say dropped, and add the one Upgrading note that matters, which
is that those migrations destroy that data and cannot be rolled back.

architecture.md still listed connector state among what the database holds; #127
removed it, so the line goes too. The README's database line never named those
tables, so it needs nothing. #123 is CI and build hardening, not a deployment
behavior, so it earns no changelog line.
davidmckayv added a commit that referenced this pull request Aug 22, 2026
* Catch the changelog and docs up to what shipped

The Unreleased notes already tracked most of the recent work, since each change
carried its own line in. This fills the gaps and fixes what went stale.

Two merged changes had no line. The address guard's alternate-encoding refusal:
it turned away the metadata and private addresses as usually written but not the
same ones spelled as an IPv6-mapped or NAT64 form, an integer, or with a trailing
dot, and it now canonicalises before it checks and refuses the container
credential endpoints even with the private-host opt-in on. And the supervisor
refusing to adopt a container it did not create, so a shared Docker host cannot
hand it a stranger's container with the computer token.

Docs that drifted: the README and the Cloud Run note still said one replica,
which the deployment doc's own Replicas section now contradicts, so both point at
the real remaining constraint instead, which is the shared browser. And
AUDIT_RETENTION_DAYS and COMPUTER_SANDBOX were configurable and documented in the
changelog and the README but missing from the configuration table.

The knowledge back-out left one more orphan the removal missed: agents/invocation.ts
routed a built-in agent to the knowledge agent that is gone, and nothing live
constructs it. Deleted with its test. The changelog line that said the local index's
connector "is going away" is now "has been removed", because it has been.

README stays a build doc; none of this adds history to it.

* Reconcile with the table drops that landed after

#126 and #127 dropped the document index and the old connector tables, so the
changelog line that said the index was "read by nothing" now understates it: the
tables are gone. Say dropped, and add the one Upgrading note that matters, which
is that those migrations destroy that data and cannot be rolled back.

architecture.md still listed connector state among what the database holds; #127
removed it, so the line goes too. The README's database line never named those
tables, so it needs nothing. #123 is CI and build hardening, not a deployment
behavior, so it earns no changelog line.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant