Document that changing the embedding model breaks an existing chunk table - #58
Conversation
Nothing warned users that the embedding model fixes the dimension of a chunk table. enable_vectorization() writes the provider's dimension into the embedding vector(N) column when it creates the table, and changing pgedge_vectorizer.model to a model of a different size afterwards leaves the worker unable to write. It handles that correctly, comparing the two dimensions before writing and marking the batch failed rather than storing anything wrong, but the documentation mentioned neither the constraint nor the recovery. There was no mention of dimensions at all in configuration.md, best_practices.md or troubleshooting.md. The correction matters more than the warning does. recreate_chunks() is the function a reader would reach for, and best_practices.md recommends it for "a complete chunk regeneration", but it cannot resolve a dimension change: it deletes the rows of the chunk table and leaves the column type untouched, so every requeued row fails exactly as before. Verified on a live table, where the column stayed vector(3) across recreate_chunks() and only became vector(5) after disable_vectorization() with drop_chunk_table and a fresh enable_vectorization(). Adds a warning admonition to the provider settings, a troubleshooting section giving the symptoms and both recovery routes, and three practice notes covering model choice, the rebuild and its provider cost. The quoted error text and every function and column named were checked against the source rather than written from memory. Also drops an em-dash from a neighbouring paragraph, per the house style.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 34 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe documentation adds guidance for selecting embedding models before vectorization and accounting for re-embedding costs. It explains that model changes can create vector-dimension mismatches because existing chunk tables are not migrated. It documents worker validation, failed queue items, diagnostic SQL, model restoration, retries, and vectorization recreation. It also clarifies worker startup detection and exponential backoff. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/best_practices.md`:
- Around line 40-43: Update the vectorizer rebuild instructions to pass
drop_chunk_table => TRUE when calling disable_vectorization() before
enable_vectorization(), ensuring the existing chunk table and embedding
vector(N) column are dropped before rebuilding with a different model dimension.
In `@docs/troubleshooting.md`:
- Around line 105-131: Clarify in the rebuilding instructions that
pgedge_vectorizer.model is global, while chunk tables are maintained per
vectorized column. State that the docs/body example repairs only that column and
the disable/enable steps must be repeated for every affected vectorized column.
- Line 16: Update the background-worker discovery statement in the
troubleshooting documentation to avoid guaranteeing detection within seconds.
State that configured databases are checked using the current
exponential-backoff interval, which can be up to five minutes after creating the
pgedge_vectorizer extension.
- Around line 80-89: Update the diagnostic SQL query in the troubleshooting
section to retain the failed-status filter while adding a condition on
error_message that matches dimension-mismatch failures. Keep the existing
grouping and selected columns so the query identifies only the failures
described by this section.
- Around line 96-103: Update the restore SQL example near the troubleshooting
guidance to use a placeholder such as previous-model-name instead of hard-coding
text-embedding-3-small, and state that it must be replaced with the model
configured before the change. Keep the retry_failed() flow unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 36f2b06b-e559-49f5-a9a2-6d1a8ad7615e
📒 Files selected for processing (3)
docs/best_practices.mddocs/configuration.mddocs/troubleshooting.md
Up to standards ✅🟢 Issues
|
Four corrections from CodeRabbit's review, all of them fair. The best practices bullet said to rebuild with disable_vectorization() and enable_vectorization() without naming drop_chunk_table. Left out, the chunk table survives, the column keeps its old dimension and the mismatch continues, so the bullet described a rebuild that does not work. The troubleshooting steps had the argument; the summary did not. The restore example hardcoded text-embedding-3-small while the prose said to restore the previous model. Anyone whose table was built by a different model would have followed it and stayed mismatched. It now carries a placeholder and points at the table=M figure in the error as the way to identify which model to go back to. The rebuild was written around a single docs/body example, but the model is one global setting while chunk tables are independent per column, so changing it strands every vectorizer whose dimension no longer matches. The steps now say to repeat them for each, and give a query listing them. The worker discovery paragraph promised detection "within seconds", which the backoff does not support once it has reached its five minute ceiling. It now states the real bound and mentions that a reload brings the check forward. That paragraph is also wrapped to the house width, having been one long line before.
2 participants
Follows the discussion on #27, where we established that the code already guards against this properly and the gap is purely documentation.
What was missing
enable_vectorization()writes the provider's dimension into the chunk table'sembedding vector(N)column when it creates the table, so the model choice is baked in permanently. Changepgedge_vectorizer.modelto a model of a different size afterwards and the worker cannot write.It handles that correctly:
src/worker.c:1512compares the two dimensions before writing, logs an actionable warning, and marks the batchfailedwithDimension mismatch: model=N, table=M. Nothing is corrupted. But the documentation mentioned neither the constraint nor the recovery, and there was no mention of dimensions at all inconfiguration.md,best_practices.mdortroubleshooting.md.The part that matters most
recreate_chunks()is the function a reader would reach for, andbest_practices.mdcurrently recommends it for "a complete chunk regeneration". It cannot resolve a dimension change. It deletes the rows of the chunk table and leaves the column type untouched, so every requeued row fails in exactly the same way and the user goes round in a circle.I verified this on a live table rather than reasoning about it:
enable_vectorization(..., embedding_dimension => 3)vector(3)recreate_chunks()vector(3)— unchangeddisable_vectorization(..., drop_chunk_table => TRUE)thenenable_vectorization()vector(5), with every row requeuedSo the docs now say plainly that
recreate_chunks()is not the answer here, and give the rebuild that is.Changes
configuration.mdgains a warning admonition beneath the provider settings table, since that is where someone reads about the model in the first place.troubleshooting.mdgains a section with the symptoms, a query to identify affected items, and both recovery routes: restoring the old model, which is right when the change was accidental, and rebuilding, which keeps the new model.best_practices.mdgains three notes covering choosing the model up front, the rebuild, and budgeting for the provider cost of re-embedding.Verification
Every claim was checked against the source rather than written from memory: all five functions named exist,
disable_vectorization()really does takedrop_chunk_table, the quotedDimension mismatch: model=N, table=Mtext matchessrc/worker.c:1527exactly, the queue columns used in the example query exist, and the documented default model matchessrc/guc.c. New prose wraps at 79 characters and carries no em-dashes; I also removed a pre-existing em-dash from a neighbouring paragraph.mkdocsis not installed here, so the site build is unverified and CI or a localmkdocs build --strictshould confirm the admonition renders. Theadmonitionextension is enabled inmkdocs.yml, though this is the first use of it in these docs.One observation, not addressed here:
best_practices.mduses bold text as section headings throughout, which the house style warns against because MkDocs can misread it as a heading and add it to the navigation pane. I added to the existing Data Management section rather than introduce another one. Worth a separate tidy-up if you agree.