Repository navigation
fix: the search postings read each document's whole text, so a term past the index's 5,000-character excerpt finds its document - #15
Conversation
…ex's 5,000-character excerpt too The search build moves unchanged to scripts/search-index.mjs, which exports it with the embedding model passed in, and scripts/build-search-index.mjs runs it. tests/search-index.test.js indexes documents with the indexer and runs the build with a stand-in model. It fails here: the postings come from the index's content field, a document's first 5,000 characters, so docs/DEVELOPMENT.md is not found by architecture, sqlite3, storage, adapter or incremental, docs/RECATALOG_GUIDE.md not by generated, autogenerated, reviewing, warnings or suggestions, a term past character 9,000 is not found, and the cut leaves the fragment arch as a term.
Reviewer's GuideThe PR separates search-index generation from its CLI wrapper and makes BM25 postings read full source documents, with a safe indexed-excerpt fallback when files cannot be read. New injectable dependencies and temporary-directory tests validate recovery of terms beyond the 5,000-character excerpt, boundary token behavior, warnings, and document-length consistency. Sequence diagram for search index generationsequenceDiagram
participant CLI as build-search-index.mjs
participant Builder as search-index.mjs
participant Postings as buildPostings
participant Embedder as createEmbedder
participant Files as Filesystem
CLI->>Builder: buildSearchIndex()
Builder->>Files: readFileSync(codex-index.json)
Builder->>Postings: buildPostings(entries)
Postings-->>Builder: docs, avgDocLength, vocabulary
Builder->>Embedder: createEmbedder()
Embedder-->>Builder: embedding function
loop each entry
Builder->>Embedder: embedder(embeddingInput, options)
Embedder-->>Builder: tensor.data
end
Builder->>Files: writeFileSync(codex-search.json)
Builder-->>CLI: searchIndex
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
The postings came from the index's content field, a document's first 5,000 characters, so a term past that point never found its document, the cut kept a fragment of the word it fell in as a term, and an edit near a guide's top pushed terms out of search: the index of #14 no longer finds docs/DEVELOPMENT.md by architecture or sqlite3, nor docs/RECATALOG_GUIDE.md by generated or reviewing. buildPostings reads each listed document under the root as scripts/auto-index.js parses it, the body after the frontmatter for Markdown and the whole file for YAML. A document it cannot read there, or whose path leads outside the root, keeps the content field's postings, with a warning naming it. The embeddings read the same text as before.
… its file README.md and docs/DEVELOPMENT.md say so, and DEVELOPMENT.md names the embedding library the build uses, @huggingface/transformers. The comment on the index's content field no longer calls it the text search reads, and coverage counts scripts/search-index.mjs beside the two other .mjs modules it names.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
PR Summary by QodoIndex whole documents for keyword search
AI Description
Diagram
High-Level Assessment
Files changed (7)
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a3d50934f5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Code Review by Qodo
1.
|
…de it keeps its excerpt's postings The indexer lists a .md or .yaml link by its own name and reads its target. The postings check the listed path, which lies under the root, then read through the link, so the whole outside file reaches the published vocabulary. This case links docs/leak.md to a file beside the root: the search must not find the file's word, must find the excerpt's, and the warning must name the link. It fails here.
…ocument, so a link under the root to a file outside it is not read The check compared the listed path, which lies under the root, and the read followed the link to its target, so a checked-in docs/leak.md linking outside the repository put the whole target in the published vocabulary. The root and the document now resolve to their real paths first; a document outside the root keeps its excerpt's postings and the warning names it. The Markdown or YAML parse follows the listed name, as the indexer's does.
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@coderabbitai review The head, 283783e, adds one change after a3d5093, which you reviewed: |
✅ Action performedReview finished.
|
|
Merged at 10:22:56 UTC as 8041bea; master's Test Suite passed on it (run 39584175180). The Build and Publish Codex Index run it started (39584174987) read every listed document in full, with no excerpt warning, and updated #14 to 4901545. On that head
Against the index at d19e45b, no document lost a word it still holds. The terms documents lost are six fragments of words the old excerpts cut (assi, eleuthe, fil, intermedia, org, sugge) and two words |
The keyword postings in
codex-search.jsoncame from the index'scontentfield, whichscripts/auto-index.jscuts at a document's first 5,000 characters. A term past that point never found its document, the cut kept a fragment of the word it fell in as a term, and an edit near a document's top pushed later terms out of search. #13 added 231 characters todocs/DEVELOPMENT.mdand 215 todocs/RECATALOG_GUIDE.mdabove the cut, and Qodo's thread on #14 names what that cost. Read against the index at d19e45b, the index of #14 (3167841):docs/DEVELOPMENT.mdlost architecture, sqlite3, storage, adapter, incremental, indexing, intelligent, better and framers. Its "SQL Cache Architecture" section held them at characters 4,765 to 4,990 and moved to 4,996 to 5,221, and the excerpt ends at "SQL Cache Arch", so it gained the term arch. It also lost manually, which ci: the search index reaches master through a pull request opened with FRAMERS_BUMP_TOKEN, since master is protected #13 took out of the guide, so that one is intended.docs/RECATALOG_GUIDE.mdlost generated, autogenerated, reviewing, warnings and suggestions. Its "Only auto-generated" line and "Reviewing Re-Catalog PRs" heading moved from characters 4,780 and 4,925 to 4,995 and 5,140, and the excerpt ends at "Only auto-gener", so it gained the term gener.sqlite3, autogenerated and reviewing left the vocabulary altogether. Frame.dev's BM25 (
lib/search/bm25.tsin framerslab/frame.dev) looks upvocabulary[term]for each query term, so a search for any of these terms stopped finding the guide that holds it. Every guide still covers every term above except manually.The change
scripts/search-index.mjs(new) holds the search build, moved fromscripts/build-search-index.mjs, which runs it (npm run build:searchis unchanged).buildSearchIndex({ root, createEmbedder })takes the embedding model as a function and loads@huggingface/transformersonly when none is passed, so a test runs the build without the model download.buildPostingsreads each listed document under the root asauto-index.jsparses it: the body after the frontmatter for Markdown, the whole file for YAML. A document's postings and its length cover its title, summary and whole text, with no cut, so no fragment term. It compares real paths before a read, so a document it cannot read under the root, whose path leads outside the root, or that is a link under the root to a file outside it, keeps thecontentfield's postings, and a warning names it.README.mdanddocs/DEVELOPMENT.mdsay the postings cover each document's whole text;DEVELOPMENT.mdnames@huggingface/transformers, the library the build uses; the comment on thecontentfield no longer calls it the text search reads; coverage countsscripts/search-index.mjs.Built from this branch by its own Build and Publish Codex Index runs (39572673413 on a3d5093, 39574155679 on 283783e, the same file),
codex-search.jsonholds 5,123 terms against #14's 2,627 and weighs 255,589 bytes against 176,829, and the average document length goes from 354.9 terms to 1,018.8; every listed document was read, with no excerpt warning. The largest document, a 93,248-character course, had its first 5% searchable.Tests, first
tests/search-index.test.js(new, 3 cases) indexes documents with the indexer, writescodex-index.jsonas it does, runs the build with a stand-in for the model, and searches thecodex-search.jsonit writes:On ca56a15, which moves the build unchanged, Test Suite run 39572303595 failed all 3 (
a search for "architecture": expected [] to include 'docs/DEVELOPMENT.md') with 303 others passing; on a3d5093, run 39572679753 passed all 306. The link case came from review: on 6ed9de6, run 39574061528 failed it (expected [] to deeply equal [ 'docs/leak.md' ], the read followed the link); on the head, 283783e, Test Suite run 39574162669 passed all 306 and rename-guard passed. Line coverage went from 30% to 32.5% and branch coverage from 75.85% to 77.53%. In the branch build'scodex-search.json,docs/DEVELOPMENT.mdis found by architecture, sqlite3, storage, adapter, incremental, indexing, intelligent, better and framers, anddocs/RECATALOG_GUIDE.mdby generated, autogenerated, reviewing, warnings and suggestions; arch and gener are gone.Reviews
After the merge
The merge changes
scripts/, so the Build and Publish Codex Index workflow runs on master and updates #14's branch with an index built this way. The two guides' entries on its new head go in a comment here, and the answer to Qodo's thread on #14 links this pull request.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation