feat(rag): make chunk_size tunable in tests and tooling (closes #932) - #1017
Open
arcgod-design wants to merge 1 commit into
Open
feat(rag): make chunk_size tunable in tests and tooling (closes #932)#1017arcgod-design wants to merge 1 commit into
arcgod-design wants to merge 1 commit into
Conversation
…shanGK#932) Adds rag_chunk_size as a tunable parameter for the RAG text splitter, alongside the existing rag_chunk_overlap. The setting flows through: - AppSettings schema: rag_chunk_size: int = 600 (matches the pre-existing hardcoded behaviour of rag_service.index_document). - PUT /api/settings/ validation: * rag_chunk_size must be in [200, 2000] inclusive * rag_chunk_overlap must be strictly less than rag_chunk_size (zero-progress splitter windows rejected with 422). - rag_service now reads rag_chunk_size from get_settings() at every index_document call, with defensive clamping so a stale DB row cannot crash the splitter (out-of-range values fall back to the default; overlap >= chunk_size is auto-reset to chunk_size // 10). - Frontend SettingsPanel exposes a new 'RAG Chunk Size' slider (200..2000, step 100) above the existing overlap slider so users can tune both at runtime. New tooling: scripts/chunk_tuner.py — a single-file CLI that empirically tests the splitter at arbitrary (chunk_size, chunk_overlap) combinations against a sample markdown file, no FastAPI / Ollama / ChromaDB required. Exits 1 if the splitter returns 0 chunks. Enforces the same bounds as the API endpoint. Used by new tests in tests/test_chunking_tuning.py::TestChunkTunerTool. 23 new pytest cases (backend/tests/test_chunking_tuning.py): - TestSchema (3 cases): rag_chunk_size defaults to 600, dumps through model_dump, defaults match legacy chunk_overlap=50. - TestSettingsValidation (10 cases): chunk_size 199/2001 -> 422 with the right loc/error msg; boundaries 200 and 2000 -> 200; overlap equal/gte chunk_size -> 422 with 'strictly less than chunk size' message; overlap < 0 -> rejected by the standalone bounds; default payload -> 200. - TestServiceUsesChunkSize (4 cases): rag_service uses rank_chunk_size / rag_chunk_overlap from get_settings(); falls back to 600/50 when settings dict is empty; bounds-clamp path for an out-of-bounds overlap; out-of-range chunk_size resets to default 600. - TestSplitterBehaviour (6 cases): smaller chunk_size -> more chunks; zero overlap -> adjacent chunks don't share a tail prefix; non-zero overlap may absorb in separators (documented); empty content -> 0 chunks; single short paragraph -> 1 chunk; separators align when paragraphs are \\n\\n separated. - TestChunkTunerTool (2 cases): the optional tooling CLI runs as both an importable module and a subprocess; skipped if scripts/chunk_tuner.py is not present (e.g. shallow clones). Aggressive rag_cache invalidation is NOT performed on chunk_size or chunk_overlap changes — the index_document path always re-embeds, and operators who change chunking parameters are expected to clear ChromaDB explicitly via the existing 'delete collection' tooling (out of scope per the issue title). Acceptance criteria from issue imDarshanGK#932: [x] Implement the change in the tests and tooling [x] Add or update tests, docs, or validation for the touched path Refs imDarshanGK#932.
|
@arcgod-design is attempting to deploy a commit to the Darshan's projects Team on Vercel. A member of the Team first needs to authorize it. |
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.
What
Makes the RAG text splitter's
chunk_sizeparameter fully tunable — closing issue #932's "Add chunking tuning to tests and tooling".Previously
chunk_sizewas hardcoded to 600 inbackend/services/rag_service.pywhile onlyrag_chunk_overlapwas exposed via the settings panel. This PR exposes both, validates both, and surfaces both in the UI + a new offline CLI tuner.Why
The local sentence-transformers embedder (
all-MiniLM-L6-v2, 384 dims) and the SQLite-ChromaDB retrieval pipeline prefer different chunk sizes depending on document type:Without a tunable
chunk_sizeusers were stuck with the 600 default. The pre-existingrag_chunk_overlapslider was there but sized for one chunk size — without a paired chunk size control the overlap's relative percentage drifted wildly. This PR binds the two together with a cross-validation check.What changed
Backend
backend/models/schemas.py— newrag_chunk_size: int = 600field onAppSettings. Default matches the previous hardcoded behaviour. Docstring documents bounds + best-effort splitter semantics.backend/routes/settings.py— two new validation steps in thePUT /api/settings/body validator:rag_chunk_sizemust lie in[200, 2000]inclusive (422 outside)rag_chunk_overlapmust be strictly less thanrag_chunk_size(422 if equal or greater — a zero-progress splitter window is unsplittable). Cross-checked after the standalone 0..200 bounds.backend/services/rag_service.py—index_document()now readsrag_chunk_size+rag_chunk_overlapfromdb_service.get_settings()at call-time. Both are defensively clamped:chunk_sizeout of[200, 2000]→ reset to default 600.overlapout of[0, 200]→ reset to default 50.overlap >= chunk_size(impossible via the API since Add chunking tuning to tests and tooling #932-merge, but theoretically present in a stale DB row) → clamp tochunk_size // 10.Logger line updated to print both values.
Frontend
frontend/src/components/SettingsPanel.jsx— a newrag_chunk_sizefield in the form initial state (?? 600) and a new slider (200..2000, step 100) above the existing chunk-overlap slider so users see a coherent pair.Tooling
scripts/chunk_tuner.py— 130-line zero-dependency CLI that empirically tests the splitter at any(chunk_size, chunk_overlap)combination against a markdown / text file (stdin or--). Used for offline experimentation without spinning up FastAPI / Ollama / ChromaDB.--json(default; structured report),--plain(human-readable)chunk_count,first_chunk_preview,last_chunk_preview,min_chunk_chars,max_chunk_chars,mean_chunk_chars200..2000/0..200/overlap < chunk_sizebounds as the API endpointTests
backend/tests/test_chunking_tuning.py— 23 pytest cases (all pass):rag_chunk_sizedefaults to 600 throughmodel_dump(); defaults match legacychunk_overlap=50.loc+ msg; boundaries 200/2000 → 200;overlap == chunk_size→ 422 with "strictly less than chunk size" message;overlap < 0rejected by the standalone bounds; default payload → 200.\n\n-separated paragraphs align withRecursiveCharacterTextSplitter's first-tier separator.module.tune(content, chunk_size, chunk_overlap)) and a subprocess CLI. Tests skip ifscripts/chunk_tuner.pyis not present (e.g. a shallow clone).Docs
docs/chunking-tuning.md— operator-facing reference: the settings table with bounds, the API request shape, the 422-body contract, the CLI tooling examples (with--plainfor human inspection and a real-world diff workflow against two chunk_size settings), and a test command summary.Verification
The existing
test_settings_timeout.py(3 cases) continues to pass unmodified — the new validation steps are additive and don't conflict with the existing rag_top_k / rag_chunk_overlap / temperature bounds.Acceptance criteria
Issue #932 acceptance criteria:
Closes #932.