Skip to content

fix: address index artifact review notes - #232

Merged
Moskize91 merged 1 commit into
mainfrom
fix/coderabbit-index-artifacts
Aug 3, 2026
Merged

fix: address index artifact review notes#232
Moskize91 merged 1 commit into
mainfrom
fix/coderabbit-index-artifacts

Conversation

@Moskize91

Copy link
Copy Markdown
Contributor

Summary

  • fix CodeRabbit review notes from the index artifact/cache PR
  • thread abort signals through embedding index jobs and validate chapter existence before index artifact writes
  • avoid redundant FTS snapshot/projection work and share embedding metadata readers
  • tighten readiness/help text, overlay locking, empty index state, and CLI test mocks

Validation

  • pnpm test:run test/cli/queue.test.ts packages/core/src/retrieval/index-artifact/build.test.ts packages/cli/src/runtime/embedding.test.ts packages/cli/src/args/help.test.ts test/cli/archive/object.test.ts packages/core/src/library/membership.test.ts
  • pnpm verify
  • pnpm test:run

@coderabbitai

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ad412a5c-2f7d-4c72-92d3-e64fab050d91

📥 Commits

Reviewing files that changed from the base of the PR and between 8d8ba2a and d397566.

📒 Files selected for processing (13)
  • packages/cli/src/args/uri/chapter/routing.ts
  • packages/cli/src/commands/library.ts
  • packages/cli/src/commands/queue/worker.ts
  • packages/cli/src/runtime/embedding.ts
  • packages/core/data/help/topics/readiness.jinja
  • packages/core/src/library/search-index.ts
  • packages/core/src/retrieval/index-artifact/build.ts
  • packages/core/src/retrieval/query/archive-view/index-state.ts
  • packages/core/src/retrieval/search-index/search/build.ts
  • packages/core/src/retrieval/search-index/search/types.ts
  • packages/core/src/storage/wikg/wikg-coordinator/flusher.ts
  • test/cli/archive/mock.ts
  • test/cli/queue.test.ts
🚧 Files skipped from review as they are similar to previous changes (13)
  • packages/core/src/retrieval/search-index/search/types.ts
  • test/cli/archive/mock.ts
  • packages/core/src/storage/wikg/wikg-coordinator/flusher.ts
  • packages/core/src/retrieval/search-index/search/build.ts
  • packages/core/data/help/topics/readiness.jinja
  • packages/cli/src/runtime/embedding.ts
  • packages/cli/src/args/uri/chapter/routing.ts
  • packages/core/src/library/search-index.ts
  • packages/core/src/retrieval/index-artifact/build.ts
  • packages/cli/src/commands/queue/worker.ts
  • packages/core/src/retrieval/query/archive-view/index-state.ts
  • test/cli/queue.test.ts
  • packages/cli/src/commands/library.ts

Summary by CodeRabbit

  • Bug Fixes

    • Improved index-building reliability when chapters are removed during processing.
    • Prevented invalid or incomplete embedding data from producing misleading search indexes.
    • Fixed archive-state updates and unsupported-action help links.
    • Improved handling of archives without searchable text or embeddings.
  • Improvements

    • Indexing progress now includes object indexing.
    • Cancelled indexing tasks stop embedding work more promptly.
    • Error messages are clearer and provider-neutral.
  • Documentation

    • Clarified when archive searches fail because required indexing artifacts are unavailable.

Walkthrough

The changes improve index-artifact reliability and state handling. Queue workers validate chapter existence and propagate cancellation signals to embedding operations. Dense-segment builders reject invalid configuration and sentence word counts. Search-index state records missing artifacts correctly. Library indexing processes streamed batches and uses shared metadata readers. State refreshes release locks reliably. CLI messages, help routes, readiness documentation, and tests are updated.

Sequence Diagram(s)

sequenceDiagram
  participant QueueWorker
  participant ChapterStore
  participant EmbeddingBuilder
  participant EmbeddingProvider
  participant ArchiveFile
  QueueWorker->>ChapterStore: verify chapter existence
  ChapterStore-->>QueueWorker: chapter revision or missing
  QueueWorker->>EmbeddingBuilder: build artifact with AbortSignal
  EmbeddingBuilder->>EmbeddingProvider: embed batches with abort signal
  EmbeddingProvider-->>EmbeddingBuilder: embeddings
  EmbeddingBuilder-->>QueueWorker: embedding artifact
  QueueWorker->>ArchiveFile: commit artifact
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title uses the required format and clearly describes fixes to the index artifact implementation.
Description check ✅ Passed The description directly summarizes the index artifact, embedding, locking, documentation, and test changes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch fix/coderabbit-index-artifacts

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@packages/core/src/library/search-index.ts`:
- Line 618: Update the compatibility error message in mergeEmbeddingState to
cover mismatched embedding models as well as provider or dimension differences,
using broader wording such as “different embedding configurations.”
🪄 Autofix (Beta)

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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c20b4869-ff95-4601-ac2e-28016f1f6414

📥 Commits

Reviewing files that changed from the base of the PR and between 81e2068 and 8d8ba2a.

📒 Files selected for processing (13)
  • packages/cli/src/args/uri/chapter/routing.ts
  • packages/cli/src/commands/library.ts
  • packages/cli/src/commands/queue/worker.ts
  • packages/cli/src/runtime/embedding.ts
  • packages/core/data/help/topics/readiness.jinja
  • packages/core/src/library/search-index.ts
  • packages/core/src/retrieval/index-artifact/build.ts
  • packages/core/src/retrieval/query/archive-view/index-state.ts
  • packages/core/src/retrieval/search-index/search/build.ts
  • packages/core/src/retrieval/search-index/search/types.ts
  • packages/core/src/storage/wikg/wikg-coordinator/flusher.ts
  • test/cli/archive/mock.ts
  • test/cli/queue.test.ts

Comment thread packages/core/src/library/search-index.ts Outdated
@Moskize91
Moskize91 force-pushed the fix/coderabbit-index-artifacts branch from 8d8ba2a to d397566 Compare August 3, 2026 23:36
@Moskize91
Moskize91 merged commit 1479fbe into main Aug 3, 2026
4 checks passed
@Moskize91
Moskize91 deleted the fix/coderabbit-index-artifacts branch August 3, 2026 23:41
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