mirror of
https://github.com/onyx-dot-app/onyx.git
synced 2026-10-21 11:00:28 -05:00
DanelegendandClaude Opus 5
e6772d6f41
Documents indexed before the bounded-sketch chunker (#16115) still hold
hundreds of row chunks each. `TABULAR_RECHUNK_ENABLED` arms a beat task
that re-chunks them in place, from the raw workbook Onyx already stores,
without re-crawling the source.
An operator turns it on by deploying with the variable set and nothing
else.
## Why a normal re-index cannot do this
`Document.content_hash()` walks Text and Image sections only, so a
tabular document's hash never moves however much the workbook changed.
The content-hash gate therefore skips every spreadsheet on any re-index,
and `force_update` — the only bypass — was wired solely into the
per-document retry path. This exposes it on `run_indexing_pipeline`;
that is the one change to shared indexing code, and it is a no-op unless
passed.
## How it picks its work
The scan walks `document` by primary key and takes only tabular
documents holding more chunks than the current chunker could produce
(`TABULAR_MAX_CHUNKS_PER_WORKBOOK` plus the large chunks multipass
derives from them — 30 by default). That bound means it skips its own
output and anything an in-flight crawl writes, so it can run during an
initial index without fighting it. Scanning by primary key keeps every
page an index scan: one pass of the table over the migration's life, not
a sort per run.
A KV record holds the cursor and a completed flag. Once the scan ends,
each later tick is one cached read, so leaving the variable set across
restarts re-runs nothing.
## Preserving what the upsert would overwrite
Re-indexing an unchanged document means `_upsert_documents_in_db` writes
the rebuilt document over a row that was already correct, so anything
the rebuild drops is destroyed corpus-wide:
- `metadata` and `title` are not kept in Postgres. They are read back
from the document's own stored chunk, as the reindex port does for
documents it cannot re-fetch.
- `doc_metadata`, owners, hierarchy and `file_id` come from the row.
Owners round-trip through their stored string form.
- `external_access` is left unset, so `includes_permissions` stays false
and the upsert leaves perm-synced ACL columns alone.
Unit tests pin each of these, plus the content-hash blindness that makes
`force_update` necessary.
## Guards
Pauses while a FUTURE index exists (the pipeline would write there
instead) or a port backfill is pending (the port copies chunks boundary
for boundary). Skips entirely under `SKIP_SPREADSHEET_CONTENT` or
`DISABLE_VECTOR_DB`. Documents whose raw file was never staged are left
out — only a crawl can rebuild those.
## Not yet verified
**This has never been run end to end.** The unit tests cover the
document rebuild and the gate reasoning; the query plan, the chunk-head
read and the delete-then-write replacement are unexercised. An
external-dependency-unit test over one real oversized document is the
obvious next step, and I'd want that before this points at a large
deployment.
Known side effects: it takes a docprocessing thread for up to 4 minutes
per 5-minute tick; every re-chunked document becomes eligible for
metadata sync; and OpenSearch reclaims disk on segment merge, so disk
lags chunk count.
## Reverting
No migration, no schema change, no data model change. Removing it is
deleting three files and reverting small hunks in thirteen; the two that
touch shared paths (`force_update`, `get_chunk_heads`) are inert on
their own.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
<!-- This is an auto-generated description by cubic. -->
---
## Summary by cubic
Adds an opt-in beat task, armed by `TABULAR_RECHUNK_ENABLED`, that
re-chunks tabular documents indexed before the bounded-sketch chunker,
in place from the workbook already in the file store, without
re-crawling the source. A KV record stops it once done, so leaving the
variable set across restarts re-runs nothing.
- The scan walks the `document` table by primary key, resolving the
cc-pair with a LATERAL so every page stays an index scan, and touches
only documents holding more chunks than the current chunker could
produce. Hidden documents are passed over, since the chunk-head read
never returns one.
- The beat entry is registered only when the variable is set, and the
task checks the completed flag before taking the lock, so a finished
migration costs one cached read per tick. Ticks run at HIGH priority
with a 4-minute expiry on the docprocessing queue.
- The re-index passes `force_update` and `skip_document_row_writes`, so
only the chunks and `chunk_count` change. Re-indexing no longer resets
admin-set `boost`/`hidden`, and ancestors resolve from
`parent_hierarchy_node_id`.
- Title and metadata are read back from the document's own stored chunk,
fetched by deterministic id; owners are restored from the row because
the chunk writer reads them off the document; Drive sheets are matched
and dispatched by stored MIME type.
- The migration's adapter re-reads `file_id` and `chunk_count` under the
document lock and aborts the batch if a crawl rewrote a document, so an
older workbook never replaces newer chunks.
- A re-chunk that fails after chunking no longer commits the smaller
count, so the document stays oversized and the next pass finds it by
rescanning; retries are bounded at three passes.
- Skips under `SKIP_SPREADSHEET_CONTENT` or `DISABLE_VECTOR_DB`; pauses
while a FUTURE index exists, while an INSTANT-swap port is backfilling,
or if the live index changes—re-checked under the document lock too,
since a swap landing mid-batch would write into the retired index. The
lock lease is extended per batch so a throttled batch can't let a second
tick start from the same cursor.
**Not yet verified**
- The migration has never completed end to end; it ran once against a
live index, which surfaced the port guard bug now fixed.
<sup>Written for commit 8cdc3ffac0.
Summary will update on new commits.</sup>
<a
href="https://cubic.dev/pr/onyx-dot-app/onyx/pull/16153?utm_source=github"
target="_blank" rel="noopener noreferrer"
data-no-image-dialog="true"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img
alt="View guided diff"
src="https://www.cubic.dev/buttons/review-in-cubic-light.svg"></picture></a>
<a
href="https://www.cubic.dev/action/auto-fix/pr/onyx-dot-app/onyx/16153?returnTo=https%3A%2F%2Fgithub.com%2Fonyx-dot-app%2Fonyx%2Fpull%2F16153&source=description"
target="_blank" rel="noopener noreferrer"
data-no-image-dialog="true"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://www.cubic.dev/buttons/turn-on-auto-fix-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://www.cubic.dev/buttons/turn-on-auto-fix-light.svg"><img
alt="Turn on auto-fix"
src="https://www.cubic.dev/buttons/turn-on-auto-fix-light.svg"></picture></a>
<!-- End of auto-generated description by cubic. -->
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **New Features**
- Added an opt-in, one-time migration to re-chunk oversized tabular
documents using existing indexed content, without requiring a source
re-crawl.
- Migration progress is saved and resumes safely across restarts, with
retries for eligible failures.
- Documents that are hidden, unavailable, or being actively changed are
skipped or retried safely.
- **Documentation**
- Added configuration guidance for enabling the tabular re-chunk
migration.
- Clarified migration behavior, completion tracking, scheduling, and
vector database requirements.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
