Add chapter index artifacts and external index caches - #231
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
Summary by CodeRabbit
WalkthroughThis change stores FTS and embedding data as chapter index artifacts in Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✨ Finishing Touches✨ Simplify code
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: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/cli/src/args/uri/archive-objects.ts (1)
190-197: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFix inconsistent
--indexeshandling for thesyncaction.For
action === "sync", the code does not callrejectArchiveFlag(action, "--indexes", ...), so--indexesis silently accepted. The returnedargsobject (Line 204-210) never includes anindexesfield for any action, so this flag has no effect when the action issync. Foraction !== "sync"(i.e.,clean), the same flag is explicitly rejected as unsupported.Passing
--indexestosynccurrently does nothing and produces no error, which misleads a user who expects it to select which artifact kinds sync. Either propagatevalues.indexesinto the sync behavior, or reject it forsyncthe same waycleanrejects it.🐛 Proposed fix to reject the unused flag consistently
if (action === "sync") { rejectStreamingJSONFlag(action, values.json, helpRoute); + rejectArchiveFlag(action, "--indexes", values.indexes, helpRoute); } else { rejectArchiveFlag(action, "--indexes", values.indexes, helpRoute); rejectArchiveBooleanFlag(action, "--jsonl", values.jsonl, helpRoute); }🤖 Prompt for 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. In `@packages/cli/src/args/uri/archive-objects.ts` around lines 190 - 197, Update the action-validation block around rejectArchiveFlag so --indexes is also rejected when action is "sync", since the returned args object does not use values.indexes. Keep the existing --jsonl validation for non-sync actions and reject the unsupported flag consistently for both sync and clean.packages/core/src/storage/wikg/wikg-coordinator/file-store.ts (1)
56-68: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winRestore
searchIndexWritebackPolicybehavior.
"archive"and"cache"remain public options used by callers and tests.markSearchIndexDatabaseDirty()ignores them, so both modes behave the same. Restore the writeback-policy gate instead of removing the option.🤖 Prompt for 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. In `@packages/core/src/storage/wikg/wikg-coordinator/file-store.ts` around lines 56 - 68, Restore handling of the searchIndexWritebackPolicy option in the file-store constructor and the markSearchIndexDatabaseDirty flow. Preserve the public "archive" and "cache" modes, store the option on the instance, and apply the policy gate so each mode retains its intended writeback behavior rather than treating both identically.packages/core/data/help/commands/predicate.jinja (1)
236-245: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the job target usage line.
setacceptsindex-fts,index-embedding-source, andindex-embedding-summary; list all sixBuildJobTargetvalues.🤖 Prompt for 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. In `@packages/core/data/help/commands/predicate.jinja` around lines 236 - 245, Update the Usage line in the `target.name == "job-target-object"` help block to list all six accepted `BuildJobTarget` values: the existing `reading-graph`, `reading-summary`, and `knowledge-graph`, plus `index-fts`, `index-embedding-source`, and `index-embedding-summary`.
🧹 Nitpick comments (14)
packages/cli/src/args/uri/chapter/routing.ts (1)
135-156: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd flag validation consistent with other chapter commands.
parseChapterIndexArtifactUriArgumentsonly rejects--input,--import,--to, and meta flags. Sibling functionparseArchiveChapterLikeArguments(lines 601-615) also rejects--digest-dir,--depth,--jsonl,--limit,--output,--output-format, and--verbose. Without the same checks here, these flags are silently accepted and ignored forget/build/deletechapter index-artifact actions, instead of producing a clear error.♻️ Proposed fix
rejectArchiveChapterMetaFlags(values, helpRoute); rejectArchiveChapterFlag("input", values.input, helpRoute); rejectArchiveChapterFlag("import", values.import, helpRoute); rejectArchiveChapterFlag("to", values.to, helpRoute); + rejectArchiveChapterFlag("digest-dir", values["digest-dir"], helpRoute); + rejectArchiveChapterFlag("depth", values.depth, helpRoute); + rejectArchiveChapterFlag("jsonl", values.jsonl, helpRoute); + rejectArchiveChapterFlag("limit", values.limit, helpRoute); + rejectArchiveChapterFlag("output", values.output, helpRoute); + rejectArchiveChapterFlag("output-format", values["output-format"], helpRoute); + if (values.verbose) { + throw new Error( + withHelpRoute( + "The chapter index artifact command does not support --verbose.", + helpRoute, + ), + ); + }🤖 Prompt for 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. In `@packages/cli/src/args/uri/chapter/routing.ts` around lines 135 - 156, Update parseChapterIndexArtifactUriArguments to reject --digest-dir, --depth, --jsonl, --limit, --output, --output-format, and --verbose using the same validation pattern as parseArchiveChapterLikeArguments, while preserving its existing checks and tail handling.packages/cli/src/commands/archive-command/inspect.ts (1)
162-168: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd improvement guidance for chapters with missing index artifacts, not just a stale cache.
createInspectImprovementsonly recommendsindex syncwhen!ftsCurrent, which fixes a stale cache built from existing artifacts.queryBlockedChapters(computed at lines 162-168) separately identifies content chapters that have neither a current FTS nor a current source-embedding artifact at all. For that case,index synccannot help; the user needs to build the missing artifacts first. Consider adding a distinct improvement entry whenqueryBlockedChapters.length > 0, pointing to the job that builds the missing FTS/embedding artifacts.Also applies to: 478-498
🤖 Prompt for 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. In `@packages/cli/src/commands/archive-command/inspect.ts` around lines 162 - 168, The inspect improvements currently handle stale FTS state but omit chapters with no current index artifacts. Update createInspectImprovements to add a distinct improvement when queryBlockedChapters.length > 0, directing users to the existing job or command that builds the missing FTS and source-embedding artifacts rather than recommending index sync.packages/cli/src/args/uri/chapter/target.ts (1)
38-42: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winHandle
"chapter-index-artifact"explicitly inclassifyArchiveUri.The current fallback returns
"object"for supported index artifacts. Add an explicitcase "chapter-index-artifact": return "object";to prevent future fallback changes from misclassifying this target.🤖 Prompt for 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. In `@packages/cli/src/args/uri/chapter/target.ts` around lines 38 - 42, Update classifyArchiveUri to add an explicit "chapter-index-artifact" case that returns "object", rather than relying on the fallback branch. Keep the existing classification behavior unchanged for all other archive URI target kinds.packages/core/src/retrieval/query/archive-view/index-state.ts (2)
153-162: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winPass the fingerprint into
writeArchiveIndexProjectionFromArtifacts.
rebuildArchiveSearchIndexalready computes the projection and fingerprint on lines 38-39. Line 158 streams every lexical row and embedding segment of every chapter a second time to compute the same value. Line 53 then builds the projection a third time for verification. Each extra pass is a full artifact scan of the archive.Accept the fingerprint as a parameter and reuse the value from the caller.
🤖 Prompt for 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. In `@packages/core/src/retrieval/query/archive-view/index-state.ts` around lines 153 - 162, Update writeArchiveIndexProjectionFromArtifacts to accept a precomputed fingerprint parameter and use it instead of calling createSearchIndexFingerprint(buildArchiveIndexProjection(document)) internally. In rebuildArchiveSearchIndex, pass the fingerprint already computed by the caller, preserving the existing projection verification behavior.
228-244: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the unreachable embedding-metadata fallbacks.
Line 161 calls
readArchiveEmbeddingState, which throws when any embedding artifact lacksmodelordimensions, independent of segment count. Therefore, when execution reaches line 228,dimensionsandmodelare always defined:
- The
segments.length > 0condition on line 229 never changes the outcome.dimensions ?? segment.vector.lengthon line 238 always resolves todimensions, which makes the check on line 240 compare a value against itself in the fallback case.model ?? ""on line 265 never yields"".Drop the fallbacks and compare
segment.vector.lengthagainstdimensionsdirectly, so a wrong-width vector is reported with the expected width.Also applies to: 265-265
🤖 Prompt for 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. In `@packages/core/src/retrieval/query/archive-view/index-state.ts` around lines 228 - 244, Remove the unreachable embedding-metadata fallbacks in the archive embedding validation flow: make the missing-metadata check unconditional for the loaded artifact, compare each segment’s vector length directly with dimensions, and replace the model ?? "" fallback near the related error reporting with model. Preserve the existing validation errors while ensuring wrong-width vectors report the defined expected dimensions.packages/core/src/retrieval/search-index/search/build.ts (1)
393-429: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared state writes.
finalizeStoredSearchIndexReplacementrepeats the transaction body offinalizeSearchIndexReplacement(lines 353-391). Only the final build-state call differs. Extract the version, fingerprint, andchaptersRevisioninserts into one private helper, so a future change to the state keys applies to both paths.🤖 Prompt for 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. In `@packages/core/src/retrieval/search-index/search/build.ts` around lines 393 - 429, Extract the repeated version, fingerprint, and chaptersRevision inserts from finalizeStoredSearchIndexReplacement and finalizeSearchIndexReplacement into a shared private helper. Have both transaction bodies call that helper, while retaining their distinct final build-state calls and existing progress behavior.packages/cli/src/commands/queue/add.ts (1)
62-88: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsolidate duplicated job-readiness validation into a shared helper.
Four locations across two files independently implement the same two readiness rules: a non-empty summary is required for
index-embedding-summary, and a current FTS artifact (sourceRevisionmatching the chapter revision) is required forknowledge-graph/reading-graph. The shared root cause is the absence of a single validator function; each copy must be kept manually in sync if the rules change.
packages/cli/src/commands/queue/add.ts#L62-L88: Extract the summary and FTS-currency checks fromaddArchiveJobsinto a shared function, e.g.assertTargetReadiness(document, chapterId, target), returning a skip reason instead of throwing.packages/cli/src/commands/queue/add.ts#L146-L175: Reuse the same shared function inassertQueueAddReady, throwing on failure instead of skipping.packages/cli/src/commands/queue/worker.ts#L156-L170: Reuse the shared FTS-currency check inexecuteGenerationBuildJobinstead of re-implementing it.packages/cli/src/commands/queue/worker.ts#L404-L434: Reuse the shared summary check inreadIndexArtifactJobSnapshotinstead of re-implementing it.🤖 Prompt for 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. In `@packages/cli/src/commands/queue/add.ts` around lines 62 - 88, Consolidate the duplicated readiness rules into a shared helper: in packages/cli/src/commands/queue/add.ts lines 62-88, extract the summary and current-FTS checks from addArchiveJobs into a validator returning a skip reason; in packages/cli/src/commands/queue/add.ts lines 146-175, have assertQueueAddReady reuse it and throw on failure; in packages/cli/src/commands/queue/worker.ts lines 156-170, replace executeGenerationBuildJob’s FTS check with the shared validation; and in packages/cli/src/commands/queue/worker.ts lines 404-434, replace readIndexArtifactJobSnapshot’s summary check with the shared validation.packages/cli/src/args/types.ts (1)
81-82: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winNarrow
indexArtifactTargetto index-only targets.The URI parser supplies only index targets, but this type still permits invalid combinations for other callers. Use an index-only union, such as
Extract<BuildJobTarget, "index-fts" | "index-embedding-source" | "index-embedding-summary">.🤖 Prompt for 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. In `@packages/cli/src/args/types.ts` around lines 81 - 82, Update the indexArtifactTarget property in the relevant argument type to accept only the index-specific BuildJobTarget values: "index-fts", "index-embedding-source", and "index-embedding-summary", using an Extract-based union or equivalent narrowing while leaving indexArtifactKind unchanged.packages/core/src/storage/wikg/wikg-coordinator/flusher.ts (1)
55-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove now-unreachable SQLite-lease-drain branch for the search-index path.
overlays(Line 55-60) unconditionally excludesSEARCH_INDEX_DATABASE_ENTRY_PATH, soentryPaths(Line 67-69) derived from it can never include that path. The checkif (entryPaths.includes(SEARCH_INDEX_DATABASE_ENTRY_PATH))at Line 84-89 is now dead code;waitForSqliteLeasesToDrainfor the search-index path will never run from this function.Confirm this drain is genuinely unnecessary now that the search-index database is never flushed into the archive, then remove the dead branch.
Also applies to: 84-89
🤖 Prompt for 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. In `@packages/core/src/storage/wikg/wikg-coordinator/flusher.ts` around lines 55 - 60, Remove the unreachable SEARCH_INDEX_DATABASE_ENTRY_PATH conditional branch from the flusher function, including its waitForSqliteLeasesToDrain call, because overlays filtering guarantees entryPaths cannot contain that path. Preserve the remaining flush logic and confirm no separate drain is required now that the search-index database is excluded from archive flushing.packages/core/src/retrieval/index-artifact/build.ts (1)
249-297: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the shared token-tier mapping.
createTextSentenceLexicalRowandcreateObjectLexicalRowbuild the sametiersobject and the same flattenedtokensarray. Extract one helper so the tier contract stays in one place.♻️ Proposed refactor
+function createTokenTiers(text: string): { + readonly tier1: string[]; + readonly tier2: string[]; + readonly tier3: string[]; +} { + const plan = createSearchTokenPlan(text); + + return { + tier1: plan.tier1.map((token) => token.encoded), + tier2: plan.tier2.map((token) => token.encoded), + tier3: plan.tier3.map((token) => token.encoded), + }; +} + function createTextSentenceLexicalRow(input: { readonly objectKind: "source-sentence" | "summary-sentence"; readonly rowPrefix: string; readonly sentence: SentenceRecord; readonly sentenceIndex: number; readonly serialId: number; }): IndexArtifactLexicalRow { const sentence = input.sentence; - const plan = createSearchTokenPlan(sentence.text); - const tiers = { - tier1: plan.tier1.map((token) => token.encoded), - tier2: plan.tier2.map((token) => token.encoded), - tier3: plan.tier3.map((token) => token.encoded), - }; + const tiers = createTokenTiers(sentence.text);🤖 Prompt for 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. In `@packages/core/src/retrieval/index-artifact/build.ts` around lines 249 - 297, Extract the duplicated tier mapping and flattened token construction from createTextSentenceLexicalRow and createObjectLexicalRow into a shared helper, then use that helper in both functions while preserving the existing tier and token values.packages/core/src/runtime/jobs/jobs.ts (1)
234-248: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd an exhaustiveness guard to the lane filter.
The switch covers every current
BuildJobTarget. If a new target is added later andnoImplicitReturnsis not enabled, the function returnsundefined, and the caller builds the SQL fragmentAND undefinedat line 225. Add aneverassertion so a new target fails at compile time.♻️ Proposed defensive change
case "index-embedding-summary": return "target = 'index-embedding-summary'"; + default: { + const unhandled: never = target; + throw new Error(`Unsupported build job target: ${String(unhandled)}`); + } } }🤖 Prompt for 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. In `@packages/core/src/runtime/jobs/jobs.ts` around lines 234 - 248, Update createBuildJobLaneFilter to add a never-based exhaustiveness assertion after the switch, ensuring any future BuildJobTarget produces a compile-time error instead of returning undefined. Preserve the existing lane filter mappings for all current targets.packages/core/data/help/commands/library-predicate.jinja (1)
152-153: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace the remaining "index rebuild" wording with
sync.Lines 153, 166, and 224 still name the concurrent operation "index rebuild". The command is now
index sync. Line 201 has the same wording. Useindex syncin these lock lists so the prose matches the command names.♻️ Proposed wording change
- - The command holds the library write lock and cannot run concurrently with `arc scan`, library `path set`, archive path changes/removes, or index rebuild for the same library. + - The command holds the library write lock and cannot run concurrently with `arc scan`, library `path set`, archive path changes/removes, or index `sync` for the same library.Also applies to: 165-166, 223-224
🤖 Prompt for 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. In `@packages/core/data/help/commands/library-predicate.jinja` around lines 152 - 153, Update the lock-list prose in the command documentation to replace every remaining “index rebuild” reference with “index sync,” including the entries near the archive-addition, line 201, and later concurrency descriptions. Keep the surrounding lock behavior and wording unchanged.packages/core/data/help/commands/predicate.jinja (1)
381-422: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSync usage omits
--jsonrejection for the archive branch.The library branch states at line 394 that
--jsonis not supported. The archive branch at lines 399-410 documentssync [--jsonl]but does not state that--jsonis rejected. Add the same note so both branches describe the same option contract.♻️ Proposed wording addition
- Free archive query can lazy-sync this cache, but explicit sync avoids the first-query delay. - With `--jsonl`, progress is emitted as line-delimited event records. + - `--json` is not supported for sync because progress is streamed.🤖 Prompt for 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. In `@packages/core/data/help/commands/predicate.jinja` around lines 381 - 422, Add a note to the archive-specific sync help branch under predicate "sync" and target.name "index-object" stating that --json is not supported because progress is streamed, matching the library branch’s option contract. Keep the existing archive sync usage and notes unchanged.packages/core/src/retrieval/query/archive-view/search/hydration.ts (1)
42-46: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the stale-cache error message into one shared constant. The new message "Wiki Graph index cache is missing or outdated. Run
<archive-uri>/index syncbefore searching." is repeated at three guard sites, so a future wording change can drift.
packages/core/src/retrieval/query/archive-view/search/hydration.ts#L42-L46: define or import the shared constant and throw it here.packages/core/src/retrieval/query/archive-view/search/hydration.ts#L74-L78: use the same shared constant.packages/core/src/retrieval/query/archive-view/search/core.ts#L228-L232: use the same shared constant.🤖 Prompt for 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. In `@packages/core/src/retrieval/query/archive-view/search/hydration.ts` around lines 42 - 46, Extract the repeated stale-cache error message into one shared constant, then use that constant when throwing from both guard sites in hydration.ts (lines 42-46 and 74-78) and core.ts (lines 228-232). Define the constant in an appropriate shared module or import it where needed, preserving the existing error behavior.
🤖 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/cli/src/args/uri/chapter/routing.ts`:
- Around line 197-204: Update the default error branch in the chapter routing
function to pass the existing helpRoute parameter to withHelpRoute instead of
the hard-coded "wg <chapter-uri>/index --help" string, preserving the computed
route used by the other branches.
In `@packages/cli/src/commands/archive-command/chapter.ts`:
- Around line 94-114: Validate the resolved chapter still exists before calling
addBuildJob in the build-index-artifact command, and repeat the same existence
validation in the worker before processing the queued job. Use the existing
chapter/document validation helpers around resolveRequiredChapterPath and the
job-processing entry point, rejecting or safely skipping deleted chapters rather
than enqueueing or executing them.
In `@packages/cli/src/commands/library.ts`:
- Line 150: Update the cache sync progress message in the library command to
include the indexing-objects step in the announced sequence, matching the phases
reported by rebuildWikiGraphLibraryIndex while preserving the existing order and
wording of the other steps.
In `@packages/cli/src/commands/queue/worker.ts`:
- Around line 404-434: Update readIndexArtifactJobSnapshot so index-fts jobs
fetch and return only the chapter revision without selecting a fragment stream
or calling listSentences. Preserve the existing sentence and summary validation
flow for embedding targets, and remove or reuse the duplicated non-empty summary
check consistently with the established validation in add.ts.
- Around line 74-81: Update executeBuildJobWithLogging to pass context into
executeIndexArtifactBuildJob, then thread context.signal through
SearchIndexEmbeddingProvider and createEmbeddingIndexArtifactInput to embedMany
via abortSignal, ensuring embedding batches honor cancellation.
In `@packages/core/data/help/topics/readiness.jinja`:
- Around line 35-40: Update the “Query behavior” readiness description to remove
the “strict mode” qualifier and state that the archive index is not ready and
ordinary queries fail when any content chapter lacks both current FTS and source
embedding artifacts; clarify that every content chapter requires at least one
current artifact.
In `@packages/core/src/document/stores/index-artifact.ts`:
- Line 1: Remove the unused getOptionalString import from the import declaration
in index-artifact.ts, while retaining getNumber and getString.
- Around line 397-421: The return values in parseStringArray and
parseNumberArray are inferred as any[] after Array.isArray; add explicit
assertions on each return so they return string[] and number[] respectively
while preserving the existing validation.
In `@packages/core/src/library/search-index.ts`:
- Around line 607-638: The embedding metadata validation is duplicated and
inconsistent between readArchiveEmbeddingState and the retrieval archive index
state path. Extract and export one shared helper from index-state.ts that
preserves the intended segment-aware fallback and validation behavior, then
replace the local readEmbeddingDimensions, readEmbeddingIdentity,
readEmbeddingModel, and readArchiveEmbeddingState logic with calls to that
helper from both cache paths.
- Around line 607-638: The embedding-state and metadata readers are duplicated
across both cache paths; consolidate them into one shared implementation. In
packages/core/src/library/search-index.ts#L607-L638, remove the local
readArchiveEmbeddingState copy and import the shared helper. In
packages/core/src/retrieval/query/archive-view/index-state.ts#L519-L561, export
readArchiveEmbeddingState, readEmbeddingDimensions, readEmbeddingIdentity, and
readEmbeddingModel as the canonical source, or move them to a shared module
under retrieval/search-index, preserving the existing validation behavior.
- Around line 647-655: Update the mismatch error thrown by mergeEmbeddingState
to refer to both embedding-source and embedding-summary artifacts, matching the
wording used by the corresponding index-state.ts validation message; leave the
dimension and embedding-configuration checks unchanged.
- Around line 476-501: Remove the archiveInput/buildArchiveIndexProjection call
from the archive processing loop, and update hasFts while iterating streamed
batch.textSentences in streamArchiveIndexProjection. Preserve the existing
non-empty text check and continue inserting each streamed record as before.
In `@packages/core/src/retrieval/index-artifact/build.ts`:
- Around line 343-371: Update the segment-building logic around
createEmbeddingSegment to enforce DENSE_SEGMENT_TARGET_WORDS >=
DENSE_SEGMENT_MIN_WORDS, or continue processing after a merge so the merge path
cannot drop remaining records if constants change. Validate
SentenceRecord.wordsCount as non-negative before segmentation so
createEmbeddingSegment never sums invalid raw values, and remove the unreachable
end <= start fallback.
In `@packages/core/src/retrieval/search-index/search/build.ts`:
- Around line 747-760: Update the indexes derivation in the search-index state
write so it considers both input.embedding and input.hasFts: preserve
"fts,dense" and "dense" for available capabilities, but store the accepted
missing-capability value when both are absent. Ensure
readSearchIndexCapabilityStatus maps that stored value to "missing" rather than
reporting a nonexistent FTS index.
In `@packages/core/src/storage/wikg/wikg-coordinator/flusher.ts`:
- Around line 128-138: Wrap the search-index branch in the flusher around
refreshOverlayArchiveState with acquireEntryLock(archiveKey,
SEARCH_INDEX_DATABASE_ENTRY_PATH, "state", ...), and perform the refresh inside
that lock. Preserve the existing requested-entry and DATABASE_ENTRY_PATH checks
while ensuring the read-modify-write cannot run concurrently with other
search-index overlay operations.
In `@test/cli/archive/mock.ts`:
- Around line 519-548: Update the direct-source mock’s
WikiGraphArchiveFile.write implementation to invoke its operation callback with
createArchiveMockDocument(), matching the existing readDocument and archive
write behavior. Ensure document-store callbacks receive the mock document rather
than no argument or undefined.
---
Outside diff comments:
In `@packages/cli/src/args/uri/archive-objects.ts`:
- Around line 190-197: Update the action-validation block around
rejectArchiveFlag so --indexes is also rejected when action is "sync", since the
returned args object does not use values.indexes. Keep the existing --jsonl
validation for non-sync actions and reject the unsupported flag consistently for
both sync and clean.
In `@packages/core/data/help/commands/predicate.jinja`:
- Around line 236-245: Update the Usage line in the `target.name ==
"job-target-object"` help block to list all six accepted `BuildJobTarget`
values: the existing `reading-graph`, `reading-summary`, and `knowledge-graph`,
plus `index-fts`, `index-embedding-source`, and `index-embedding-summary`.
In `@packages/core/src/storage/wikg/wikg-coordinator/file-store.ts`:
- Around line 56-68: Restore handling of the searchIndexWritebackPolicy option
in the file-store constructor and the markSearchIndexDatabaseDirty flow.
Preserve the public "archive" and "cache" modes, store the option on the
instance, and apply the policy gate so each mode retains its intended writeback
behavior rather than treating both identically.
---
Nitpick comments:
In `@packages/cli/src/args/types.ts`:
- Around line 81-82: Update the indexArtifactTarget property in the relevant
argument type to accept only the index-specific BuildJobTarget values:
"index-fts", "index-embedding-source", and "index-embedding-summary", using an
Extract-based union or equivalent narrowing while leaving indexArtifactKind
unchanged.
In `@packages/cli/src/args/uri/chapter/routing.ts`:
- Around line 135-156: Update parseChapterIndexArtifactUriArguments to reject
--digest-dir, --depth, --jsonl, --limit, --output, --output-format, and
--verbose using the same validation pattern as parseArchiveChapterLikeArguments,
while preserving its existing checks and tail handling.
In `@packages/cli/src/args/uri/chapter/target.ts`:
- Around line 38-42: Update classifyArchiveUri to add an explicit
"chapter-index-artifact" case that returns "object", rather than relying on the
fallback branch. Keep the existing classification behavior unchanged for all
other archive URI target kinds.
In `@packages/cli/src/commands/archive-command/inspect.ts`:
- Around line 162-168: The inspect improvements currently handle stale FTS state
but omit chapters with no current index artifacts. Update
createInspectImprovements to add a distinct improvement when
queryBlockedChapters.length > 0, directing users to the existing job or command
that builds the missing FTS and source-embedding artifacts rather than
recommending index sync.
In `@packages/cli/src/commands/queue/add.ts`:
- Around line 62-88: Consolidate the duplicated readiness rules into a shared
helper: in packages/cli/src/commands/queue/add.ts lines 62-88, extract the
summary and current-FTS checks from addArchiveJobs into a validator returning a
skip reason; in packages/cli/src/commands/queue/add.ts lines 146-175, have
assertQueueAddReady reuse it and throw on failure; in
packages/cli/src/commands/queue/worker.ts lines 156-170, replace
executeGenerationBuildJob’s FTS check with the shared validation; and in
packages/cli/src/commands/queue/worker.ts lines 404-434, replace
readIndexArtifactJobSnapshot’s summary check with the shared validation.
In `@packages/core/data/help/commands/library-predicate.jinja`:
- Around line 152-153: Update the lock-list prose in the command documentation
to replace every remaining “index rebuild” reference with “index sync,”
including the entries near the archive-addition, line 201, and later concurrency
descriptions. Keep the surrounding lock behavior and wording unchanged.
In `@packages/core/data/help/commands/predicate.jinja`:
- Around line 381-422: Add a note to the archive-specific sync help branch under
predicate "sync" and target.name "index-object" stating that --json is not
supported because progress is streamed, matching the library branch’s option
contract. Keep the existing archive sync usage and notes unchanged.
In `@packages/core/src/retrieval/index-artifact/build.ts`:
- Around line 249-297: Extract the duplicated tier mapping and flattened token
construction from createTextSentenceLexicalRow and createObjectLexicalRow into a
shared helper, then use that helper in both functions while preserving the
existing tier and token values.
In `@packages/core/src/retrieval/query/archive-view/index-state.ts`:
- Around line 153-162: Update writeArchiveIndexProjectionFromArtifacts to accept
a precomputed fingerprint parameter and use it instead of calling
createSearchIndexFingerprint(buildArchiveIndexProjection(document)) internally.
In rebuildArchiveSearchIndex, pass the fingerprint already computed by the
caller, preserving the existing projection verification behavior.
- Around line 228-244: Remove the unreachable embedding-metadata fallbacks in
the archive embedding validation flow: make the missing-metadata check
unconditional for the loaded artifact, compare each segment’s vector length
directly with dimensions, and replace the model ?? "" fallback near the related
error reporting with model. Preserve the existing validation errors while
ensuring wrong-width vectors report the defined expected dimensions.
In `@packages/core/src/retrieval/query/archive-view/search/hydration.ts`:
- Around line 42-46: Extract the repeated stale-cache error message into one
shared constant, then use that constant when throwing from both guard sites in
hydration.ts (lines 42-46 and 74-78) and core.ts (lines 228-232). Define the
constant in an appropriate shared module or import it where needed, preserving
the existing error behavior.
In `@packages/core/src/retrieval/search-index/search/build.ts`:
- Around line 393-429: Extract the repeated version, fingerprint, and
chaptersRevision inserts from finalizeStoredSearchIndexReplacement and
finalizeSearchIndexReplacement into a shared private helper. Have both
transaction bodies call that helper, while retaining their distinct final
build-state calls and existing progress behavior.
In `@packages/core/src/runtime/jobs/jobs.ts`:
- Around line 234-248: Update createBuildJobLaneFilter to add a never-based
exhaustiveness assertion after the switch, ensuring any future BuildJobTarget
produces a compile-time error instead of returning undefined. Preserve the
existing lane filter mappings for all current targets.
In `@packages/core/src/storage/wikg/wikg-coordinator/flusher.ts`:
- Around line 55-60: Remove the unreachable SEARCH_INDEX_DATABASE_ENTRY_PATH
conditional branch from the flusher function, including its
waitForSqliteLeasesToDrain call, because overlays filtering guarantees
entryPaths cannot contain that path. Preserve the remaining flush logic and
confirm no separate drain is required now that the search-index database is
excluded from archive flushing.
🪄 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: 592a400c-a23a-4a55-b2f6-1523a61d8f3e
📒 Files selected for processing (90)
README.mddocs/en/wikg-standard.mddocs/zh-CN/wikg-standard.mdpackages/cli/src/args/archive-index.test.tspackages/cli/src/args/archive.test.tspackages/cli/src/args/help.test.tspackages/cli/src/args/help.tspackages/cli/src/args/helper/actions.tspackages/cli/src/args/helper/chapter/normalize.tspackages/cli/src/args/helper/parse.tspackages/cli/src/args/parse.tspackages/cli/src/args/types.tspackages/cli/src/args/uri/archive-objects.tspackages/cli/src/args/uri/chapter/routing.tspackages/cli/src/args/uri/chapter/target.tspackages/cli/src/args/uri/entry.tspackages/cli/src/args/uri/library.tspackages/cli/src/commands/archive-command/chapter.tspackages/cli/src/commands/archive-command/index.tspackages/cli/src/commands/archive-command/inspect.tspackages/cli/src/commands/archive-command/run/document.test.tspackages/cli/src/commands/archive-command/run/document.tspackages/cli/src/commands/archive-command/run/uri.test.tspackages/cli/src/commands/archive-command/search-index.tspackages/cli/src/commands/library.tspackages/cli/src/commands/queue/add.tspackages/cli/src/commands/queue/estimate.tspackages/cli/src/commands/queue/index.tspackages/cli/src/commands/queue/worker.tspackages/cli/src/runtime/planning.tspackages/core/data/help/commands/library-predicate.jinjapackages/core/data/help/commands/library.jinjapackages/core/data/help/commands/predicate.jinjapackages/core/data/help/commands/uri.jinjapackages/core/data/help/topics/config.jinjapackages/core/data/help/topics/library.jinjapackages/core/data/help/topics/readiness.jinjapackages/core/data/help/topics/recipe.jinjapackages/core/data/help/topics/uri.jinjapackages/core/src/document/directory/core.tspackages/core/src/document/directory/index-artifact-invalidation.test.tspackages/core/src/document/directory/types.tspackages/core/src/document/index.tspackages/core/src/document/schema.tspackages/core/src/document/stores/index-artifact.test.tspackages/core/src/document/stores/index-artifact.tspackages/core/src/document/stores/index.tspackages/core/src/document/stores/types.tspackages/core/src/document/types.tspackages/core/src/graph/knowledge-build/commit.tspackages/core/src/graph/reading-build/artifact.tspackages/core/src/index.tspackages/core/src/library/membership.test.tspackages/core/src/library/search-index.tspackages/core/src/retrieval/index-artifact/build.test.tspackages/core/src/retrieval/index-artifact/build.tspackages/core/src/retrieval/index-artifact/index.tspackages/core/src/retrieval/query/archive-view/index-state.test.tspackages/core/src/retrieval/query/archive-view/index-state.tspackages/core/src/retrieval/query/archive-view/index.tspackages/core/src/retrieval/query/archive-view/search/core.tspackages/core/src/retrieval/query/archive-view/search/hydration.tspackages/core/src/retrieval/query/index.tspackages/core/src/retrieval/search-index/index.tspackages/core/src/retrieval/search-index/search/build.tspackages/core/src/retrieval/search-index/search/core.tspackages/core/src/retrieval/search-index/search/settings.tspackages/core/src/retrieval/search-index/search/types.tspackages/core/src/runtime/jobs/jobs.tspackages/core/src/runtime/jobs/row.tspackages/core/src/runtime/jobs/schema.tspackages/core/src/runtime/jobs/types.tspackages/core/src/storage/wikg/archive/constants.tspackages/core/src/storage/wikg/archive/document-files.tspackages/core/src/storage/wikg/archive/write.tspackages/core/src/storage/wikg/wikg-coordinator/file-store.tspackages/core/src/storage/wikg/wikg-coordinator/flusher.tspackages/core/src/storage/wikg/wikg-coordinator/overlays.tspackages/core/src/text/summary-build/artifact.tspackages/core/src/text/summary-build/snapshot/empty-stores.tspackages/core/src/text/summary-build/snapshot/index.tstest/cli/archive/chapter.test.tstest/cli/archive/mock.tstest/cli/archive/object.test.tstest/cli/queue.test.tstest/core/retrieval/query/archive-view/helpers.tstest/core/retrieval/query/archive-view/index.test.tstest/core/runtime/gc/gc.test.tstest/core/storage/wikg/archive.test.tstest/core/storage/wikg/wiki-graph-archive-file.test.ts
💤 Files with no reviewable changes (4)
- packages/core/src/retrieval/search-index/search/settings.ts
- packages/core/src/storage/wikg/archive/write.ts
- packages/core/src/storage/wikg/archive/constants.ts
- packages/cli/src/args/helper/parse.ts
Comment on lines
+197
to
+204
| default: | ||
| throw new Error( | ||
| withHelpRoute( | ||
| `The chapter index artifact does not support \`${action}\`. Use get, build, or delete.`, | ||
| "wg <chapter-uri>/index --help", | ||
| ), | ||
| ); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix the hard-coded help route in the default branch.
Line 201 passes a literal string "wg <chapter-uri>/index --help" to withHelpRoute instead of the helpRoute parameter used everywhere else in this function. <chapter-uri> is not substituted, so the error message tells the user to run a non-functional placeholder command instead of the actual computed route.
🐛 Proposed fix
default:
throw new Error(
withHelpRoute(
`The chapter index artifact does not support \`${action}\`. Use get, build, or delete.`,
- "wg <chapter-uri>/index --help",
+ helpRoute,
),
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| default: | |
| throw new Error( | |
| withHelpRoute( | |
| `The chapter index artifact does not support \`${action}\`. Use get, build, or delete.`, | |
| "wg <chapter-uri>/index --help", | |
| ), | |
| ); | |
| } | |
| default: | |
| throw new Error( | |
| withHelpRoute( | |
| `The chapter index artifact does not support \`${action}\`. Use get, build, or delete.`, | |
| helpRoute, | |
| ), | |
| ); |
🤖 Prompt for 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.
In `@packages/cli/src/args/uri/chapter/routing.ts` around lines 197 - 204, Update
the default error branch in the chapter routing function to pass the existing
helpRoute parameter to withHelpRoute instead of the hard-coded "wg
<chapter-uri>/index --help" string, preserving the computed route used by the
other branches.
Comment on lines
+94
to
+114
| case "build-index-artifact": { | ||
| const chapterId = await readArchiveDocument( | ||
| args.path, | ||
| async (document) => | ||
| await resolveRequiredChapterPath(document, args.chapterPath), | ||
| ); | ||
| const job = await addBuildJob({ | ||
| archivePath: args.path, | ||
| chapterId, | ||
| target: requireIndexArtifactTarget(args.indexArtifactTarget), | ||
| }); | ||
|
|
||
| if (args.json === true) { | ||
| await writeTextToStdout(formatCLIJSON(job)); | ||
| return; | ||
| } | ||
| await writeTextToStdout( | ||
| `Queued ${job.target} job ${job.jobId} for chapter ${job.chapterId}.\n`, | ||
| ); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect addBuildJob and assertNoActiveBuildJobConflicts/assertNoActiveBuildJobs implementations.
rg -n "export.{0,20}function addBuildJob|export const addBuildJob" -A 40 packages/core/src
rg -n "function assertNoActiveBuildJobConflicts|function assertNoActiveBuildJobs" -A 20 packages/cli/srcRepository: oomol-lab/wiki-graph
Length of output: 3299
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- addBuildJob implementation and related helpers ---'
sed -n '1,180p' packages/core/src/runtime/jobs/jobs.ts
printf '%s\n' '--- conflict guard definitions and usages ---'
rg -n "assertNoActiveBuildJobConflicts|assertNoActiveBuildJobs" packages
printf '%s\n' '--- chapter command context ---'
sed -n '1,180p' packages/cli/src/commands/archive-command/chapter.ts
printf '%s\n' '--- chapter resolution and archive mutation APIs ---'
rg -n "resolveRequiredChapterPath|writeArchiveDocument|readArchiveDocument" packages/core packages/cliRepository: oomol-lab/wiki-graph
Length of output: 17745
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- conflict implementation ---'
cat -n packages/core/src/runtime/jobs/conflicts.ts
printf '%s\n' '--- document session implementation ---'
cat -n packages/cli/src/commands/archive-command/run/document.ts
printf '%s\n' '--- remaining addBuildJob helpers and job execution references ---'
rg -n "findActiveBuildJobInLane|mergeActiveBuildJob|chapterId|resolve.*Chapter|require.*Chapter" packages/core/src/runtime/jobs packages/core/src/runtime -g '*.ts' -A 18 -B 5
printf '%s\n' '--- archive document locking or revision handling ---'
rg -n "lock|revision|transaction|open.*Document|close|readArchiveDocument|writeArchiveDocument" packages/cli/src/commands/archive-command/run packages/core/src/document -g '*.ts' -A 12 -B 4Repository: oomol-lab/wiki-graph
Length of output: 50377
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- index-artifact target mapping ---'
rg -n "requireIndexArtifactTarget|IndexArtifactTarget|index-fts|index-embedding" packages/cli/src/commands/archive-command/chapter.ts packages/core/src -g '*.ts' -A 12 -B 4
printf '%s\n' '--- build worker execution path ---'
rg -n "executeJob|run.*BuildJob|BuildJobExecutionContext|readArchive|chapterId" packages/core/src/runtime -g '*.ts' -A 10 -B 5 | head -n 500
printf '%s\n' '--- build-job and chapter command tests ---'
rg -n "build-index-artifact|addBuildJob|assertNoActiveBuild|index artifact|active.*job" packages/cli packages/core -g '*test*.ts' -A 12 -B 5
printf '%s\n' '--- archive read/write locking implementation references ---'
rg -n "class WikiGraphArchiveFile|readDocument|async write\\(|lock|revision" packages/core/src -g '*.ts' -A 15 -B 5Repository: oomol-lab/wiki-graph
Length of output: 50377
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- exact target helper ---'
rg -n "function requireIndexArtifactTarget|requireIndexArtifactTarget" packages/cli/src/commands/archive-command/chapter.ts -A 20 -B 5
printf '%s\n' '--- archive class locations ---'
rg -l "class WikiGraphArchiveFile|readDocument\\(" packages/core/src -g '*.ts'
printf '%s\n' '--- worker entry points ---'
rg -l "assertBuildJobInputRevision|executeJob\\(|BuildJobExecutionContext" packages/core/src -g '*.ts'
printf '%s\n' '--- focused tests ---'
rg -l "build-index-artifact|addBuildJob|index artifact" packages/cli packages/core -g '*test*.ts' | head -n 80Repository: oomol-lab/wiki-graph
Length of output: 2595
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- archive file read/write methods ---'
rg -n "class WikiGraphArchiveFile|readDocument|write\\(" packages/core/src/storage/wikg/wiki-graph-archive-file.ts -A 35 -B 8
printf '%s\n' '--- worker execution ---'
cat -n packages/core/src/runtime/jobs/worker.ts | sed -n '1,260p'
printf '%s\n' '--- build artifact execution APIs ---'
rg -n "build.*artifact|indexArtifacts|chapterId" packages/core/src/retrieval/index-artifact packages/core/src/worker.ts packages/core/src/runtime/jobs -g '*.ts' -A 12 -B 5
printf '%s\n' '--- chapter ID and removal semantics ---'
rg -n "async function removeChapter|function removeChapter|removeChapter\\(|serials\\.delete|delete.*chapter|next.*serial|chapterId" packages/core/src/document packages/core/src -g '*.ts' -A 12 -B 5 | head -n 500Repository: oomol-lab/wiki-graph
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- archive readDocument continuation ---'
sed -n '30,150p' packages/core/src/storage/wikg/wiki-graph-archive-file.ts
printf '%s\n' '--- archive session coordinator ---'
rg -n "withArchiveSession|materializeReadWorkspace|withEntryLock" packages/core/src/storage/wikg/wikg-coordinator -g '*.ts' -A 35 -B 8
printf '%s\n' '--- all job executor wiring ---'
rg -n "runBuildJobWorker|executeJob:|executeJob\\s*=|index-fts|index-embedding-source|index-embedding-summary" packages -g '*.ts' -A 20 -B 8 | head -n 800
printf '%s\n' '--- removeChapter implementation ---'
rg -n "export async function removeChapter|export function removeChapter" packages/core/src/document -g '*.ts' -A 100 -B 5Repository: oomol-lab/wiki-graph
Length of output: 50377
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- job target dispatch references ---'
rg -n "job\\.target|target ===|switch \\(.*target|BuildJobTarget" packages -g '*.ts' -A 15 -B 8 | head -n 1000
printf '%s\n' '--- chapter mutation exports and serial deletion ---'
rg -n "removeChapter|deleteSerial|removeSerial|serials\\.|chapters" packages/core/src/document -g '*.ts' -A 30 -B 8 | head -n 1000
printf '%s\n' '--- build-job schema and addBuildJob tests ---'
cat -n packages/core/src/runtime/jobs/schema.ts
rg -n "addBuildJob|findActiveBuildJobInLane|active_index|active_reading|mergeActiveBuildJob" packages/core/src -g '*test*.ts' -A 25 -B 8
printf '%s\n' '--- command behavior documentation ---'
rg -n "build-index-artifact|index-artifact|active build|queued.*job" README.md docs packages -g '*.md' -g '*.ts' -A 12 -B 4 | head -n 800Repository: oomol-lab/wiki-graph
Length of output: 50377
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- files defining chapter mutations ---'
rg -l "removeChapter|moveChapter" packages/core/src -g '*.ts'
printf '%s\n' '--- mutation definitions ---'
for f in $(rg -l "removeChapter|moveChapter" packages/core/src -g '*.ts' | head -n 20); do
printf '\n### %s\n' "$f"
rg -n "removeChapter|moveChapter" "$f" -A 80 -B 8
done
printf '%s\n' '--- concise behavioral source verifier ---'
python3 - <<'PY'
from pathlib import Path
jobs = Path("packages/core/src/runtime/jobs/jobs.ts").read_text()
conflicts = Path("packages/core/src/runtime/jobs/conflicts.ts").read_text()
chapter = Path("packages/cli/src/commands/archive-command/chapter.ts").read_text()
checks = {
"addBuildJob_inserts_options_chapter_id_without_archive_lookup":
"options.chapterId" in jobs and "requireChapter" not in jobs,
"addBuildJob_deduplicates_same_target_lane":
"AND chapter_id = ?" in jobs and "AND ${laneFilter}" in jobs,
"conflict_guard_matches_all_active_states":
"state IN ('queued', 'running', 'canceling', 'paused')" in conflicts,
"build_index_command_calls_addBuildJob":
"await addBuildJob({" in chapter,
"build_index_command_has_no_conflict_guard":
chapter[chapter.index('case "build-index-artifact"'):chapter.index('case "delete-index-artifact"')].count("assertNoActiveBuild") == 0,
}
for name, value in checks.items():
print(f"{name}={value}")
PYRepository: oomol-lab/wiki-graph
Length of output: 16248
Validate chapter existence before enqueueing.
addBuildJob stores chapterId and only merges jobs in the same lane. It does not validate that the chapter exists. A concurrent removal can therefore create a queued job for a deleted chapter. Add validation when enqueueing and in the worker. ``
🤖 Prompt for 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.
In `@packages/cli/src/commands/archive-command/chapter.ts` around lines 94 - 114,
Validate the resolved chapter still exists before calling addBuildJob in the
build-index-artifact command, and repeat the same existence validation in the
worker before processing the queued job. Use the existing chapter/document
validation helpers around resolveRequiredChapterPath and the job-processing
entry point, rejecting or safely skipping deleted chapters rather than
enqueueing or executing them.
| json: { type: "started" }, | ||
| kind: "lifecycle", | ||
| text: `library index enable started\nindexes: ${buildOptions.indexes ?? "auto"}\nsteps: ${formatIndexEnableSteps(buildOptions).join(" -> ")}`, | ||
| text: "library index cache sync started\nsteps: collecting -> clearing -> indexing-text -> indexing-dense -> finalizing", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
List the indexing-objects step.
rebuildWikiGraphLibraryIndex reports the indexing-objects phase (packages/core/src/library/search-index.ts line 524). The announced step list omits it, so users see a phase that the command did not announce.
🐛 Proposed fix
- text: "library index cache sync started\nsteps: collecting -> clearing -> indexing-text -> indexing-dense -> finalizing",
+ text: "library index cache sync started\nsteps: collecting -> clearing -> indexing-text -> indexing-objects -> indexing-dense -> finalizing",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| text: "library index cache sync started\nsteps: collecting -> clearing -> indexing-text -> indexing-dense -> finalizing", | |
| text: "library index cache sync started\nsteps: collecting -> clearing -> indexing-text -> indexing-objects -> indexing-dense -> finalizing", |
🤖 Prompt for 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.
In `@packages/cli/src/commands/library.ts` at line 150, Update the cache sync
progress message in the library command to include the indexing-objects step in
the announced sequence, matching the phases reported by
rebuildWikiGraphLibraryIndex while preserving the existing order and wording of
the other steps.
Comment on lines
+74
to
+81
| ): Promise<void> { | ||
| if (isIndexArtifactBuildTarget(job.target)) { | ||
| await executeIndexArtifactBuildJob(job, reporter); | ||
| return; | ||
| } | ||
|
|
||
| await executeGenerationBuildJob(job, reporter, context); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether buildSearchIndexEmbeddingProvider/embedMany accept an abort signal.
rg -n -A 15 'function buildSearchIndexEmbeddingProvider' packages/cli/src/runtime/embedding.ts
rg -n 'abortSignal|signal' packages/cli/src/runtime/embedding.tsRepository: oomol-lab/wiki-graph
Length of output: 842
🏁 Script executed:
#!/bin/bash
set -u
echo '--- worker.ts relevant sections ---'
sed -n '60,90p;320,450p' packages/cli/src/commands/queue/worker.ts
echo '--- embedding.ts relevant sections ---'
sed -n '1,150p' packages/cli/src/runtime/embedding.ts
echo '--- embedding provider and input call sites ---'
rg -n -A 20 -B 8 'createEmbeddingIndexArtifactInput|embedTexts|embedMany|buildSearchIndexEmbeddingProvider' packages
echo '--- package metadata for AI SDK versions ---'
rg -n -A 3 -B 3 '"(ai|`@ai-sdk/`[^"]+)"' package.json packages/*/package.json pnpm-lock.yaml yarn.lock package-lock.json 2>/dev/null || trueRepository: oomol-lab/wiki-graph
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -u
echo '--- all signal/cancellation references in queue worker ---'
rg -n -A 8 -B 8 'context\.signal|assertJobStillRunning|cancel|executeIndexArtifactBuildJob|executeGenerationBuildJob' packages/cli/src/commands/queue/worker.ts packages/cli/src
echo '--- embedding input implementation ---'
rg -n -A 80 -B 15 'function createEmbeddingIndexArtifactInput|const createEmbeddingIndexArtifactInput|createEmbeddingIndexArtifactInput' packages
echo '--- local type declarations for SearchIndexEmbeddingProvider ---'
rg -n -A 20 -B 10 'interface SearchIndexEmbeddingProvider|type SearchIndexEmbeddingProvider|SearchIndexEmbeddingProvider' packages
echo '--- lockfile entries for AI SDK ---'
rg -n -A 8 -B 2 '(^|/)(ai|`@ai-sdk/`)' pnpm-lock.yaml yarn.lock package-lock.json 2>/dev/null || trueRepository: oomol-lab/wiki-graph
Length of output: 50377
🌐 Web query:
AI SDK embedMany abortSignal options embedMany documentation
💡 Result:
The embedMany function in the Vercel AI SDK supports an optional abortSignal parameter, which allows you to cancel an ongoing embedding process or enforce a timeout [1][2][3]. The parameter accepts a standard Web API AbortSignal object [1][3]. A common use case is implementing a timeout using AbortSignal.timeout [1][3]. Example usage: import { embedMany } from 'ai'; import { openai } from '@ai-sdk/openai'; const { embeddings } = await embedMany({ model: openai.embeddingModel('text-embedding-3-small'), values: ['text 1', 'text 2'], abortSignal: AbortSignal.timeout(5000), // Aborts after 5 seconds }); In addition to abortSignal, embedMany also supports other configuration options such as maxRetries, maxParallelCalls, and headers [2][4]. Detailed documentation for these parameters can be found in the AI SDK Core reference [5][6].
Citations:
- 1: https://ai-sdk.dev/docs/ai-sdk-core/embeddings
- 2: https://github.com/vercel/ai/blob/258c0933/packages/ai/src/embed/embed-many.ts
- 3: https://vercel-ai.mintlify.app/ai-sdk-core/embeddings
- 4: https://ai-sdk.dev/v4/docs/ai-sdk-core/embeddings
- 5: https://ai-sdk.dev/docs/reference/ai-sdk-core/embed-many
- 6: https://ai-sdk.dev/docs/reference/ai-sdk-core
🏁 Script executed:
#!/bin/bash
set -u
echo '--- locate index-artifact implementation ---'
fd -t f -i 'index-artifact' packages/core/src
rg -l 'createEmbeddingIndexArtifactInput' packages/core/src
echo '--- createEmbeddingIndexArtifactInput implementation ---'
file="$(rg -l 'createEmbeddingIndexArtifactInput' packages/core/src | head -n 1)"
echo "FILE=$file"
[ -n "$file" ] && rg -n -A 100 -B 15 'createEmbeddingIndexArtifactInput' "$file"
echo '--- SearchIndexEmbeddingProvider type ---'
type_file="$(rg -l 'interface SearchIndexEmbeddingProvider' packages/core/src | head -n 1)"
echo "FILE=$type_file"
[ -n "$type_file" ] && sed -n '45,75p' "$type_file"
echo '--- BuildJobExecutionContext and worker cancellation lifecycle ---'
rg -n -A 35 -B 15 'BuildJobExecutionContext|runBuildJobWorker|AbortController|abort\(' packages/core/src packages/cli/srcRepository: oomol-lab/wiki-graph
Length of output: 35271
🏁 Script executed:
#!/bin/bash
set -u
echo '--- package manifests and lockfile AI SDK versions ---'
for f in package.json packages/cli/package.json pnpm-lock.yaml yarn.lock package-lock.json; do
if [ -f "$f" ]; then
echo "FILE=$f"
rg -n -A 4 -B 4 '"ai"|"`@ai-sdk/`' "$f" || true
fi
done
echo '--- embedding tests for abort propagation ---'
rg -n -A 25 -B 10 'abort|AbortSignal|signal' packages/cli/src/runtime/embedding.test.ts packages/core/src/retrieval packages/cli/src/commands/queue
echo '--- queue worker tests ---'
fd -t f -i 'worker' packages/cli/src/commands/queueRepository: oomol-lab/wiki-graph
Length of output: 8947
🏁 Script executed:
#!/bin/bash
set -u
echo '--- index-artifact build implementation ---'
sed -n '1,180p' packages/core/src/retrieval/index-artifact/build.ts
sed -n '470,545p' packages/core/src/retrieval/search-index/search/build.ts
echo '--- cancellation and embedding facts verifier ---'
python3 - <<'PY'
from pathlib import Path
worker = Path("packages/cli/src/commands/queue/worker.ts").read_text()
embedding = Path("packages/cli/src/runtime/embedding.ts").read_text()
provider_type = Path("packages/core/src/retrieval/search-index/search/types.ts").read_text()
jobs = Path("packages/core/src/runtime/jobs/types.ts").read_text()
assert "executeIndexArtifactBuildJob(job, reporter);" in worker
assert "executeGenerationBuildJob(job, reporter, context);" in worker
assert "embedTexts: async (texts)" in embedding
assert "await embedMany({" in embedding
embed_many_call = embedding.split("await embedMany({", 1)[1].split("});", 1)[0]
assert "abortSignal" not in embed_many_call
assert "embedTexts(texts: readonly string[])" in provider_type
assert "readonly signal: AbortSignal" in jobs
print("routing_drops_context=true")
print("embedding_embedMany_abortSignal_forwarded=false")
print("provider_embedTexts_accepts_signal=false")
print("worker_context_contains_abort_signal=true")
PYRepository: oomol-lab/wiki-graph
Length of output: 8044
Thread cancellation through embedding index builds.
executeBuildJobWithLogging drops context for index-artifact jobs. The embedding loop does not pass context.signal to embedMany or check cancellation between batches. Pass the signal through SearchIndexEmbeddingProvider and createEmbeddingIndexArtifactInput to embedMany({ abortSignal: context.signal }).
🤖 Prompt for 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.
In `@packages/cli/src/commands/queue/worker.ts` around lines 74 - 81, Update
executeBuildJobWithLogging to pass context into executeIndexArtifactBuildJob,
then thread context.signal through SearchIndexEmbeddingProvider and
createEmbeddingIndexArtifactInput to embedMany via abortSignal, ensuring
embedding batches honor cancellation.
Comment on lines
+404
to
+434
| async function readIndexArtifactJobSnapshot(job: BuildJob): Promise<{ | ||
| readonly revision: number; | ||
| readonly sentences: readonly SentenceRecord[]; | ||
| }> { | ||
| return await new WikiGraphArchiveFile(job.archivePath).readDocument( | ||
| async (document) => { | ||
| const revision = await document.serials.getRevision(job.chapterId); | ||
| const stream = | ||
| job.target === "index-embedding-summary" | ||
| ? document.getSummaryFragments(job.chapterId) | ||
| : document.getSerialFragments(job.chapterId); | ||
| if (job.target === "index-embedding-summary") { | ||
| const summary = await document.readSummary(job.chapterId); | ||
| if (summary === undefined || summary.trim() === "") { | ||
| throw new Error( | ||
| `Chapter ${job.chapterId} has no summary. Build a reading summary before building a summary embedding index artifact.`, | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| if (stream.listSentences === undefined) { | ||
| throw new Error("Text stream does not expose sentence listing."); | ||
| } | ||
|
|
||
| return { | ||
| revision, | ||
| sentences: await stream.listSentences(), | ||
| }; | ||
| }, | ||
| ); | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Avoid loading the full sentence list for "index-fts" jobs.
readIndexArtifactJobSnapshot always calls document.getSerialFragments(job.chapterId) (or getSummaryFragments) and stream.listSentences(), then returns sentences. For job.target === "index-fts", the caller only uses snapshot.revision; snapshot.sentences is discarded, and replaceChapterFtsIndexArtifact re-reads the chapter content itself to build the artifact. This means the full sentence list is materialized twice for every FTS job, with the index-fts planning profile assuming up to 100,000 words per call. Skip the sentence-listing work for "index-fts" and fetch only the revision.
⚡ Proposed fix
async function readIndexArtifactJobSnapshot(job: BuildJob): Promise<{
readonly revision: number;
readonly sentences: readonly SentenceRecord[];
}> {
return await new WikiGraphArchiveFile(job.archivePath).readDocument(
async (document) => {
const revision = await document.serials.getRevision(job.chapterId);
+ if (job.target === "index-fts") {
+ return { revision, sentences: [] };
+ }
const stream =
job.target === "index-embedding-summary"
? document.getSummaryFragments(job.chapterId)
: document.getSerialFragments(job.chapterId);This function also duplicates the non-empty-summary check used in packages/cli/src/commands/queue/add.ts; see the consolidated comment.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async function readIndexArtifactJobSnapshot(job: BuildJob): Promise<{ | |
| readonly revision: number; | |
| readonly sentences: readonly SentenceRecord[]; | |
| }> { | |
| return await new WikiGraphArchiveFile(job.archivePath).readDocument( | |
| async (document) => { | |
| const revision = await document.serials.getRevision(job.chapterId); | |
| const stream = | |
| job.target === "index-embedding-summary" | |
| ? document.getSummaryFragments(job.chapterId) | |
| : document.getSerialFragments(job.chapterId); | |
| if (job.target === "index-embedding-summary") { | |
| const summary = await document.readSummary(job.chapterId); | |
| if (summary === undefined || summary.trim() === "") { | |
| throw new Error( | |
| `Chapter ${job.chapterId} has no summary. Build a reading summary before building a summary embedding index artifact.`, | |
| ); | |
| } | |
| } | |
| if (stream.listSentences === undefined) { | |
| throw new Error("Text stream does not expose sentence listing."); | |
| } | |
| return { | |
| revision, | |
| sentences: await stream.listSentences(), | |
| }; | |
| }, | |
| ); | |
| } | |
| async function readIndexArtifactJobSnapshot(job: BuildJob): Promise<{ | |
| readonly revision: number; | |
| readonly sentences: readonly SentenceRecord[]; | |
| }> { | |
| return await new WikiGraphArchiveFile(job.archivePath).readDocument( | |
| async (document) => { | |
| const revision = await document.serials.getRevision(job.chapterId); | |
| if (job.target === "index-fts") { | |
| return { revision, sentences: [] }; | |
| } | |
| const stream = | |
| job.target === "index-embedding-summary" | |
| ? document.getSummaryFragments(job.chapterId) | |
| : document.getSerialFragments(job.chapterId); | |
| if (job.target === "index-embedding-summary") { | |
| const summary = await document.readSummary(job.chapterId); | |
| if (summary === undefined || summary.trim() === "") { | |
| throw new Error( | |
| `Chapter ${job.chapterId} has no summary. Build a reading summary before building a summary embedding index artifact.`, | |
| ); | |
| } | |
| } | |
| if (stream.listSentences === undefined) { | |
| throw new Error("Text stream does not expose sentence listing."); | |
| } | |
| return { | |
| revision, | |
| sentences: await stream.listSentences(), | |
| }; | |
| }, | |
| ); | |
| } |
🤖 Prompt for 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.
In `@packages/cli/src/commands/queue/worker.ts` around lines 404 - 434, Update
readIndexArtifactJobSnapshot so index-fts jobs fetch and return only the chapter
revision without selecting a fragment stream or calling listSentences. Preserve
the existing sentence and summary validation flow for embedding targets, and
remove or reuse the duplicated non-empty summary check consistently with the
established validation in add.ts.
Comment on lines
+647
to
+655
| if ( | ||
| current.dimensions !== next.dimensions || | ||
| current.model !== next.model || | ||
| current.identity !== next.identity | ||
| ) { | ||
| throw new Error( | ||
| "Source embedding artifacts use different embedding providers or dimensions; rebuild them with one embeddings configuration.", | ||
| ); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the mismatch error text.
mergeEmbeddingState merges both embedding-source and embedding-summary artifacts. The message names only source artifacts, so an operator can look at the wrong artifact kind. Report both kinds, as index-state.ts line 553 does.
🐛 Proposed fix
throw new Error(
- "Source embedding artifacts use different embedding providers or dimensions; rebuild them with one embeddings configuration.",
+ "Embedding artifacts use different embedding providers or dimensions; rebuild them with one embeddings configuration.",
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if ( | |
| current.dimensions !== next.dimensions || | |
| current.model !== next.model || | |
| current.identity !== next.identity | |
| ) { | |
| throw new Error( | |
| "Source embedding artifacts use different embedding providers or dimensions; rebuild them with one embeddings configuration.", | |
| ); | |
| } | |
| if ( | |
| current.dimensions !== next.dimensions || | |
| current.model !== next.model || | |
| current.identity !== next.identity | |
| ) { | |
| throw new Error( | |
| "Embedding artifacts use different embedding providers or dimensions; rebuild them with one embeddings configuration.", | |
| ); | |
| } |
🤖 Prompt for 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.
In `@packages/core/src/library/search-index.ts` around lines 647 - 655, Update the
mismatch error thrown by mergeEmbeddingState to refer to both embedding-source
and embedding-summary artifacts, matching the wording used by the corresponding
index-state.ts validation message; leave the dimension and
embedding-configuration checks unchanged.
Comment on lines
+343
to
+371
| if (end <= start) { | ||
| end = start + 1; | ||
| wordsCount = Math.max(0, records[start]!.wordsCount); | ||
| } | ||
|
|
||
| const segmentRecords = records.slice(start, end); | ||
| const segment = createEmbeddingSegment(segmentRecords, segments.length); | ||
|
|
||
| if (segment.wordsCount < DENSE_SEGMENT_MIN_WORDS && segments.length > 0) { | ||
| const previous = segments.pop()!; | ||
| const mergedRecords = records.filter( | ||
| (record) => | ||
| record.sentenceIndex >= previous.startSentenceIndex && | ||
| record.sentenceIndex <= segment.endSentenceIndex, | ||
| ); | ||
|
|
||
| segments.push(createEmbeddingSegment(mergedRecords, segments.length)); | ||
| break; | ||
| } | ||
|
|
||
| segments.push(segment); | ||
|
|
||
| if (end >= records.length) { | ||
| break; | ||
| } | ||
| const nextStart = findSegmentOverlapStart(records, start, end); | ||
|
|
||
| start = nextStart <= start ? end : nextStart; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Read the DENSE_SEGMENT_* constants to confirm the target/min relationship.
fd -t f -e ts . packages/core/src/retrieval/search-index | xargs rg -n 'DENSE_SEGMENT_(MIN|MAX|TARGET|OVERLAP)_WORDS\s*='Repository: oomol-lab/wiki-graph
Length of output: 583
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- build.ts ---'
sed -n '280,385p' packages/core/src/retrieval/index-artifact/build.ts
printf '%s\n' '--- segment constants and references ---'
rg -n -C 3 'DENSE_SEGMENT_(MIN|MAX|TARGET|OVERLAP)_WORDS|findSegmentOverlapStart|createEmbeddingSegment' packages/core/src/retrieval
printf '%s\n' '--- relevant tests ---'
rg -n -C 4 'DENSE_SEGMENT_MIN_WORDS|short segment|segment.*merge|embedding segment|index artifact' packages/core --glob '*test*' --glob '*spec*'Repository: oomol-lab/wiki-graph
Length of output: 19879
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- segment helpers ---'
sed -n '379,430p' packages/core/src/retrieval/index-artifact/build.ts
printf '%s\n' '--- behavioral probe ---'
python3 - <<'PY'
from pathlib import Path
import re
source = Path("packages/core/src/retrieval/index-artifact/build.ts").read_text()
types = Path("packages/core/src/retrieval/search-index/search/types.ts").read_text()
values = {
name: int(value)
for name, value in re.findall(
r"export const (DENSE_SEGMENT_(?:MIN|MAX|TARGET|OVERLAP)_WORDS)\s*=\s*(\d+)",
types,
)
}
print("constants:", values)
def find_overlap(records, start, end, overlap):
words = 0
for index in range(end - 1, start, -1):
words += max(0, records[index])
if words >= overlap:
return index
return end
def segment(records, target, minimum, maximum, overlap):
segments = []
start = 0
while start < len(records):
end = start
words_count = 0
while end < len(records):
next_words = max(0, records[end])
if (
end > start
and words_count >= minimum
and words_count + next_words > maximum
):
break
words_count += next_words
end += 1
if words_count >= target:
break
if end <= start:
end = start + 1
words_count = max(0, records[start])
current = records[start:end]
current_words = sum(max(0, x) for x in current)
if current_words < minimum and segments:
previous = segments.pop()
merged = records[previous[0]:end]
segments.append((previous[0], end, sum(max(0, x) for x in merged)))
break
segments.append((start, end, current_words))
if end >= len(records):
break
next_start = find_overlap(records, start, end, overlap)
start = end if next_start <= start else next_start
return segments
current = values
print("current config:", segment(
[100, 100, 100, 100, 100, 100, 100],
current["DENSE_SEGMENT_TARGET_WORDS"],
current["DENSE_SEGMENT_MIN_WORDS"],
current["DENSE_SEGMENT_MAX_WORDS"],
current["DENSE_SEGMENT_OVERLAP_WORDS"],
))
hypothetical = dict(current, DENSE_SEGMENT_TARGET_WORDS=50)
print("target < minimum:", segment(
[50, 50, 50, 50, 50, 50],
hypothetical["DENSE_SEGMENT_TARGET_WORDS"],
hypothetical["DENSE_SEGMENT_MIN_WORDS"],
hypothetical["DENSE_SEGMENT_MAX_WORDS"],
hypothetical["DENSE_SEGMENT_OVERLAP_WORDS"],
))
PYRepository: oomol-lab/wiki-graph
Length of output: 1494
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- segment helpers ---'
sed -n '379,430p' packages/core/src/retrieval/index-artifact/build.ts
printf '%s\n' '--- behavioral probe ---'
python3 - <<'PY'
from pathlib import Path
import re
types = Path("packages/core/src/retrieval/search-index/search/types.ts").read_text()
values = {
name: int(value)
for name, value in re.findall(
r"export const (DENSE_SEGMENT_(?:MIN|MAX|TARGET|OVERLAP)_WORDS)\s*=\s*(\d+)",
types,
)
}
print("constants:", values)
def find_overlap(records, start, end, overlap):
words = 0
for index in range(end - 1, start, -1):
words += max(0, records[index])
if words >= overlap:
return index
return end
def segment(records, target, minimum, maximum, overlap):
segments = []
start = 0
while start < len(records):
end = start
words_count = 0
while end < len(records):
next_words = max(0, records[end])
if end > start and words_count >= minimum and words_count + next_words > maximum:
break
words_count += next_words
end += 1
if words_count >= target:
break
if end <= start:
end = start + 1
words_count = max(0, records[start])
current_words = sum(max(0, x) for x in records[start:end])
if current_words < minimum and segments:
previous = segments.pop()
merged = records[previous[0]:end]
segments.append((previous[0], end, sum(max(0, x) for x in merged)))
break
segments.append((start, end, current_words))
if end >= len(records):
break
next_start = find_overlap(records, start, end, overlap)
start = end if next_start <= start else next_start
return segments
print("current config:", segment([100] * 7, values["DENSE_SEGMENT_TARGET_WORDS"],
values["DENSE_SEGMENT_MIN_WORDS"], values["DENSE_SEGMENT_MAX_WORDS"],
values["DENSE_SEGMENT_OVERLAP_WORDS"]))
print("target < minimum:", segment([50] * 6, 50,
values["DENSE_SEGMENT_MIN_WORDS"], values["DENSE_SEGMENT_MAX_WORDS"],
values["DENSE_SEGMENT_OVERLAP_WORDS"]))
PYRepository: oomol-lab/wiki-graph
Length of output: 1494
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- SentenceRecord definition ---'
rg -n -C 8 'interface SentenceRecord|type SentenceRecord|SentenceRecord\s*=' packages/core/src
printf '%s\n' '--- wordsCount producers and validation ---'
rg -n -C 5 'wordsCount' packages/core/src/document packages/core/src/retrieval --glob '*.ts'Repository: oomol-lab/wiki-graph
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- normal word-count producer ---'
sed -n '1,45p' packages/core/src/document/fragments/sentence.ts
printf '%s\n' '--- embedding-segment callers ---'
sed -n '165,215p' packages/core/src/retrieval/index-artifact/build.ts
rg -n -C 5 'listTextStreamSentences|build.*Embedding|createEmbeddingIndexArtifactInput|replace.*Embedding' packages/core/src/retrieval/index-artifact packages/core/src --glob '*.ts' | head -240
printf '%s\n' '--- sentence-file parser ---'
sed -n '50,85p' packages/core/src/document/fragments/file.tsRepository: oomol-lab/wiki-graph
Length of output: 23645
Enforce the segment-size invariants. DENSE_SEGMENT_TARGET_WORDS is 300 and DENSE_SEGMENT_MIN_WORDS is 80, so the target-below-minimum case is not current. If these constants change, line 360 can drop all remaining records. Enforce DENSE_SEGMENT_TARGET_WORDS >= DENSE_SEGMENT_MIN_WORDS or continue after the merge. Validate non-negative SentenceRecord.wordsCount values because the loop clamps them, but createEmbeddingSegment sums raw values. Remove the unreachable end <= start fallback.
🤖 Prompt for 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.
In `@packages/core/src/retrieval/index-artifact/build.ts` around lines 343 - 371,
Update the segment-building logic around createEmbeddingSegment to enforce
DENSE_SEGMENT_TARGET_WORDS >= DENSE_SEGMENT_MIN_WORDS, or continue processing
after a merge so the merge path cannot drop remaining records if constants
change. Validate SentenceRecord.wordsCount as non-negative before segmentation
so createEmbeddingSegment never sums invalid raw values, and remove the
unreachable end <= start fallback.
Comment on lines
+747
to
+760
| const indexes = | ||
| input.embedding === undefined | ||
| ? "fts" | ||
| : input.hasFts | ||
| ? "fts,dense" | ||
| : "dense"; | ||
|
|
||
| await database.run( | ||
| ` | ||
| INSERT INTO search_index_state(key, value) | ||
| VALUES ('indexes', ?) | ||
| `, | ||
| [indexes], | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Record no lexical capability when hasFts is false and no embedding exists.
When input.embedding is undefined and input.hasFts is false, the code stores indexes = "fts". Callers pass hasFts from artifact presence, so an archive without FTS artifacts and without embeddings is reported as having an FTS index. readSearchIndexCapabilityStatus then returns "fts", and the CLI prints Enabled indexes: fts for a cache that holds no searchable rows.
Derive the value from both flags.
🐛 Proposed fix
- const indexes =
- input.embedding === undefined
- ? "fts"
- : input.hasFts
- ? "fts,dense"
- : "dense";
+ const indexes = input.hasFts
+ ? input.embedding === undefined
+ ? "fts"
+ : "fts,dense"
+ : input.embedding === undefined
+ ? "missing"
+ : "dense";Confirm that "missing" is the accepted stored value for an empty cache, or use the value that readSearchIndexCapabilityStatus maps to "missing".
🤖 Prompt for 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.
In `@packages/core/src/retrieval/search-index/search/build.ts` around lines 747 -
760, Update the indexes derivation in the search-index state write so it
considers both input.embedding and input.hasFts: preserve "fts,dense" and
"dense" for available capabilities, but store the accepted missing-capability
value when both are absent. Ensure readSearchIndexCapabilityStatus maps that
stored value to "missing" rather than reporting a nonexistent FTS index.
Comment on lines
+128
to
+138
| if ( | ||
| requestedEntryPaths?.has(SEARCH_INDEX_DATABASE_ENTRY_PATH) === true && | ||
| currentOverlays.some( | ||
| (overlay) => overlay.entryPath === DATABASE_ENTRY_PATH, | ||
| ) | ||
| ) { | ||
| await refreshOverlayArchiveState( | ||
| archiveKey, | ||
| SEARCH_INDEX_DATABASE_ENTRY_PATH, | ||
| ); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Acquire the entry lock before calling refreshOverlayArchiveState for the search-index path.
refreshOverlayArchiveState reads and re-upserts the SEARCH_INDEX_DATABASE_ENTRY_PATH overlay here without holding a "state" lock for that entry path. Elsewhere in this codebase (WikgDocumentFileStore#resolveSqliteDatabasePath, #readOverlay, deleteFile), every read-modify-write of an entry overlay is wrapped in withEntryLock/acquireEntryLock(archiveKey, entryPath, "state", ...) to prevent races.
Because SEARCH_INDEX_DATABASE_ENTRY_PATH is excluded from overlays at Line 55-60, no lock is acquired for it in the loop at Line 74-79. A concurrent process resolving or adopting the search-index cache overlay (e.g., #resolveSqliteDatabasePath) could interleave with this unlocked read-then-write, corrupting or discarding the just-refreshed overlay state.
🔒 Proposed fix to guard the refresh with the entry lock
if (
requestedEntryPaths?.has(SEARCH_INDEX_DATABASE_ENTRY_PATH) === true &&
currentOverlays.some(
(overlay) => overlay.entryPath === DATABASE_ENTRY_PATH,
)
) {
- await refreshOverlayArchiveState(
- archiveKey,
- SEARCH_INDEX_DATABASE_ENTRY_PATH,
- );
+ const releaseSearchIndexLock = await acquireEntryLock(
+ archiveKey,
+ SEARCH_INDEX_DATABASE_ENTRY_PATH,
+ "state",
+ );
+ try {
+ await refreshOverlayArchiveState(
+ archiveKey,
+ SEARCH_INDEX_DATABASE_ENTRY_PATH,
+ );
+ } finally {
+ await releaseSearchIndexLock();
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if ( | |
| requestedEntryPaths?.has(SEARCH_INDEX_DATABASE_ENTRY_PATH) === true && | |
| currentOverlays.some( | |
| (overlay) => overlay.entryPath === DATABASE_ENTRY_PATH, | |
| ) | |
| ) { | |
| await refreshOverlayArchiveState( | |
| archiveKey, | |
| SEARCH_INDEX_DATABASE_ENTRY_PATH, | |
| ); | |
| } | |
| if ( | |
| requestedEntryPaths?.has(SEARCH_INDEX_DATABASE_ENTRY_PATH) === true && | |
| currentOverlays.some( | |
| (overlay) => overlay.entryPath === DATABASE_ENTRY_PATH, | |
| ) | |
| ) { | |
| const releaseSearchIndexLock = await acquireEntryLock( | |
| archiveKey, | |
| SEARCH_INDEX_DATABASE_ENTRY_PATH, | |
| "state", | |
| ); | |
| try { | |
| await refreshOverlayArchiveState( | |
| archiveKey, | |
| SEARCH_INDEX_DATABASE_ENTRY_PATH, | |
| ); | |
| } finally { | |
| await releaseSearchIndexLock(); | |
| } | |
| } |
🤖 Prompt for 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.
In `@packages/core/src/storage/wikg/wikg-coordinator/flusher.ts` around lines 128
- 138, Wrap the search-index branch in the flusher around
refreshOverlayArchiveState with acquireEntryLock(archiveKey,
SEARCH_INDEX_DATABASE_ENTRY_PATH, "state", ...), and perform the refresh inside
that lock. Preserve the existing requested-entry and DATABASE_ENTRY_PATH checks
while ensuring the read-modify-write cannot run concurrently with other
search-index overlay operations.
Comment on lines
+519
to
+548
| isArchiveSearchIndexCurrent: vi.fn(() => | ||
| Promise.resolve(archiveMockState.ftsCurrent), | ||
| ), | ||
| rebuildArchiveSearchIndex: vi.fn(() => Promise.resolve()), | ||
| resolveWikiGraphLibraryArchivePath: vi.fn((uri: string) => { | ||
| const archiveId = uri.split("/").at(-1) ?? "archive"; | ||
|
|
||
| return Promise.resolve(`/tmp/library/${archiveId}.wikg`); | ||
| }), | ||
| WikiGraphArchiveFile: class { | ||
| readonly #path: string; | ||
|
|
||
| public constructor(path: string) { | ||
| this.#path = path; | ||
| } | ||
|
|
||
| public async readDocument( | ||
| operation: (document: unknown) => Promise<unknown>, | ||
| ): Promise<unknown> { | ||
| archiveMockState.readCalls.push(this.#path); | ||
| return await operation(createArchiveMockDocument()); | ||
| } | ||
|
|
||
| public async write( | ||
| operation: (document: unknown) => Promise<unknown>, | ||
| ): Promise<unknown> { | ||
| archiveMockState.writeCalls.push(this.#path); | ||
| return await operation(createArchiveMockDocument()); | ||
| } | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 8 'WikiGraphArchiveFile' test/cli/archive/mock.tsRepository: oomol-lab/wiki-graph
Length of output: 1204
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- mock.ts ---'
sed -n '490,610p' test/cli/archive/mock.ts
printf '%s\n' '--- WikiGraphArchiveFile references ---'
rg -n -C 4 'WikiGraphArchiveFile|readDocument|\.write\(' test/cli/archive test/cli --glob '*.ts'Repository: oomol-lab/wiki-graph
Length of output: 14256
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- imports of the mocked modules ---'
rg -n -C 3 'from ["'\"'']wiki-graph-core|from ["'\"''][^"'\"'']*wiki-graph-archive-file\.js|import\(["'\"'']wiki-graph-core|import\(["'\"''][^"'\"'']*wiki-graph-archive-file\.js' . --glob '*.ts' --glob '*.tsx'
printf '%s\n' '--- archive mock consumers ---'
rg -n -C 3 'from ["'\"'']\./mock\.js|from ["'\"'']\./mock|from ["'\"''].*archive/mock' test --glob '*.ts' --glob '*.tsx'
printf '%s\n' '--- write callback implementations and call sites ---'
rg -n -C 5 'public async write|\.write\(|write\(' packages test --glob '*.ts' --glob '*.tsx'Repository: oomol-lab/wiki-graph
Length of output: 318
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- wiki-graph-core imports ---'
rg -n -C 3 'wiki-graph-core' packages test --glob '*.ts' --glob '*.tsx'
printf '%s\n' '--- direct archive-file imports ---'
rg -n -C 3 'wiki-graph-archive-file\.js' packages test --glob '*.ts' --glob '*.tsx'
printf '%s\n' '--- archive mock consumers ---'
rg -n -C 3 'archive/mock|from "./mock|from "../mock' test --glob '*.ts' --glob '*.tsx'
printf '%s\n' '--- write callback implementations and call sites ---'
rg -n -C 5 'public async write|\.write\(' packages test --glob '*.ts' --glob '*.tsx'Repository: oomol-lab/wiki-graph
Length of output: 50377
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
archive_file="$(fd -t f -i 'wiki-graph-archive-file' packages/core/src)"
printf '%s\n' '--- archive file candidates ---'
printf '%s\n' "$archive_file"
printf '%s\n' '--- archive file implementation ---'
for file in $archive_file; do
sed -n '1,260p' "$file"
done
printf '%s\n' '--- direct source imports in production code ---'
rg -n -C 3 'storage/wikg/wiki-graph-archive-file|WikiGraphArchiveFile' packages/cli/src packages/core/src --glob '*.ts'
printf '%s\n' '--- archive command write call sites ---'
rg -n -C 8 'new WikiGraphArchiveFile|\.write\(' packages/cli/src/commands/archive-command packages/cli/src/commands/object-metadata.ts packages/cli/src/commands/queue --glob '*.ts'Repository: oomol-lab/wiki-graph
Length of output: 50376
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- archive mock overrides ---'
sed -n '514,760p' test/cli/archive/mock.ts
printf '%s\n' '--- core functions that write through WikiGraphArchiveFile ---'
rg -n -C 10 'new WikiGraphArchiveFile\(.*\)\.write|\.write\(\s*$' packages/core/src/library packages/core/src/api packages/core/src/storage --glob '*.ts'
printf '%s\n' '--- archive tests calling likely core write paths ---'
rg -n -C 6 'finalizeWikiGraphLibraryArchiveWrite|rebuildWikiGraphLibraryIndex|replaceChapterFtsIndexArtifact|writeArchiveDocument|runArchiveCommand' test/cli/archive packages/cli/src/commands/archive-command --glob '*.ts'Repository: oomol-lab/wiki-graph
Length of output: 50377
🏁 Script executed:
python3 - <<'PY'
from pathlib import Path
import re
path = Path("test/cli/archive/mock.ts")
text = path.read_text()
factories = list(re.finditer(r'vi\.mock\(', text))
archive_factories = [
m for m in factories
if text[m.start(): text.find(");", m.start()) + 2].find("WikiGraphArchiveFile") >= 0
]
print(f"WikiGraphArchiveFile mock factories: {len(archive_factories)}")
for index, match in enumerate(archive_factories, 1):
start = match.start()
end = text.find("\n\n", start)
block = text[start:] if end < 0 else text[start:end]
write_match = re.search(r'public async write\((.*?)\): Promise<unknown> \{(.*?)\n \}', block, re.S)
print(f"factory_{index}_start_line={text.count(chr(10), 0, start) + 1}")
if write_match:
signature, body = write_match.groups()
print(f"write_signature={signature.strip()}")
print(f"write_calls_operation_with_document={'operation(createArchiveMockDocument())' in body}")
print(f"write_calls_operation_without_arguments={'operation()' in body}")
PYRepository: oomol-lab/wiki-graph
Length of output: 222
🏁 Script executed:
python3 - <<'PY'
from pathlib import Path
import re
path = Path("test/cli/archive/mock.ts")
lines = path.read_text().splitlines()
ranges = [(514, 550), (552, 584)]
for start, end in ranges:
block = "\n".join(lines[start - 1:end])
print(f"range={start}-{end}")
print(f"contains_vi_mock={block.lstrip().startswith('vi.mock(')}")
print(f"contains_wikigrapharchivefile={block.count('WikiGraphArchiveFile')}")
write_match = re.search(
r"public async write\((.*?)\): Promise<unknown> \{(.*?)\n\s*\}",
block,
re.S,
)
if write_match:
signature, body = write_match.groups()
print(f"write_signature={signature.strip()}")
print(f"calls_operation_with_document={'operation(createArchiveMockDocument())' in body}")
print(f"calls_operation_without_arguments={'operation()' in body}")
PYRepository: oomol-lab/wiki-graph
Length of output: 562
Pass the mock document to the direct-source write callback.
These are separate vi.mock factories, not shadowed properties. The direct-source mock calls operation() without a document, although WikiGraphArchiveFile.write passes one. Pass createArchiveMockDocument() to prevent document-store callbacks from receiving undefined.
🤖 Prompt for 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.
In `@test/cli/archive/mock.ts` around lines 519 - 548, Update the direct-source
mock’s WikiGraphArchiveFile.write implementation to invoke its operation
callback with createArchiveMockDocument(), matching the existing readDocument
and archive write behavior. Ensure document-store callbacks receive the mock
document rather than no argument or undefined.
This was referenced Aug 3, 2026
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
1 participant
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.
Summary
Validation