Fix knowledge graph progress output#92
Conversation
Summary by CodeRabbit
WalkthroughThis PR replaces fragment-based topology grouping, chunk retrieval, and chapter-summary logic with sentence-start-index/segment-based equivalents. The build queue gains per-job cache and log directories persisted in schema and used across CLI, progress reporting, and GC. Progress output introduces metric groups replacing token-specific fields, with a new "committing" phase. LLM request logging is centralized via new formatting helpers. WIKG archives now embed and validate a mutation token, and the coordinator can adopt cached search-index overlays across archives when mutation tokens match, alongside expanded GC cleanup of descendant directories. Sequence Diagram(s)sequenceDiagram
participant CLI as cli/queue.ts
participant Reporter as ProgressReporter
participant Archive as Archive write
CLI->>Reporter: updatePhase(committing, done=0)
CLI->>Archive: commit artifact
Archive-->>CLI: write complete
CLI->>Reporter: updatePhase(committing, done=1)
sequenceDiagram
participant Coordinator as wikg-coordinator
participant State as entry_overlays (state DB)
participant Archive as Target archive workspace
Coordinator->>State: listSearchIndexAdoptionCandidates(mutationToken)
State-->>Coordinator: matching candidate overlays
Coordinator->>Coordinator: validate eligibility (locks, mutation token, no conflicts)
Coordinator->>Archive: move candidate sqlite file into target workspace
Coordinator->>State: update entry_overlays with new path
Compact metadata
Related issues: None referenced in provided data. Related PRs: None referenced in provided data. Suggested labels: refactor, feature, storage-schema, needs-careful-review Suggested reviewers: Reviewers familiar with topology/grouping internals, build-queue persistence, and WIKG archive format. Poem 🚥 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/facade/build-queue.ts (1)
199-211: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftAdd a migration for
build_jobs.CREATE TABLE IF NOT EXISTSwon’t addcache_path/log_pathto an existing queue DB, so upgraded installs will still hit missing-column failures inmapBuildJob. Add anALTER TABLE/backfill step for existing rows.🤖 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 `@src/facade/build-queue.ts` around lines 199 - 211, The build queue schema update for build_jobs is incomplete because CREATE TABLE IF NOT EXISTS will not add the new cache_path and log_path columns to existing databases. Update the build-queue initialization/migration logic around BUILD_QUEUE_SCHEMA_SQL and mapBuildJob to detect older queue DBs, run an ALTER TABLE/backfill step for build_jobs, and ensure existing rows get valid values before they are read.src/wikg/wikg-coordinator.ts (1)
597-627: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDry-run should include descendant-directory counts.
--dry-runis user-facing here, so the GC report should still scan and count disposable descendant directories like the file-removal loop above; gating the wholeremoveDisposableDescendantDirectories()call on!context.dryRunmakes dry runs underreportscanned/removed/freedBytes.🤖 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 `@src/wikg/wikg-coordinator.ts` around lines 597 - 627, The dry-run path in the GC flow is skipping descendant-directory accounting, so the report undercounts scanned, removed, and freedBytes. Update the archive cleanup logic in wikg-coordinator’s workspace scan to always call removeDisposableDescendantDirectories for counting purposes, and only gate the actual destructive deletion behavior on context.dryRun, matching the file-removal loop’s dry-run handling.
🧹 Nitpick comments (5)
test/topology/grouping.test.ts (1)
61-86: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated fixture helper across test files.
createSerialFragmentshere is nearly identical to the one intest/topology/segment-incision.test.ts(lines 167-192), and a similar inline fixture exists intest/topology/topology.test.ts. Consider extracting to a shared test util to avoid drift as the segment-based fixture evolves.🤖 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/topology/grouping.test.ts` around lines 61 - 86, The createSerialFragments test fixture is duplicated across topology test files, so update the grouping.test.ts helper to use a shared reusable test utility instead of maintaining a near-identical local copy. Extract the common fixture logic from createSerialFragments into a shared topology test helper that can be imported by grouping.test.ts, segment-incision.test.ts, and the similar inline setup in topology.test.ts, keeping the same ReadonlySerialFragments shape and behavior.src/llm/client.ts (2)
365-376: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant error logging across
[[Error]]and[[Error Stack]]blocks.Both the payment-required and non-retryable branches now append a
[[Error]]:block viaformatError(...)and immediately follow it withformatRequestResultLog(tokenUsage, error), which appends its own[[Error Stack]]:block for the same error. This isn't a bug, but it duplicates error context (message chain vs. stack trace) in every failure log. Consider dropping the standalone[[Error]]:append and relying solely onformatRequestResultLog's usage+error-stack composition, or vice versa, to keep logs concise.♻️ Example consolidation
- await requestLog.append( - `[[Error]]:\n${formatError(paymentError)}\n\n`, - ); - await requestLog.append(formatRequestResultLog(tokenUsage, error)); + await requestLog.append(formatRequestResultLog(tokenUsage, paymentError)); throw paymentError;Also applies to: 609-617
🤖 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 `@src/llm/client.ts` around lines 365 - 376, The failure paths in `src/llm/client.ts` are logging the same error twice: once through the standalone `[[Error]]` append and again inside `formatRequestResultLog(tokenUsage, error)` via `[[Error Stack]]`. Update the payment-required and non-retryable branches to keep only one error representation, using either the direct `formatError(...)` append or the `formatRequestResultLog` output, and apply the same cleanup to the other matching block referenced in the comment.
609-651: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a more type-safe usage/cache-hit representation.
formatRequestUsageLog/formatRequestResultLogmixLanguageModelUsagewith a"cache-hit"string sentinel in the same parameter type. A small discriminated shape (e.g.,{ kind: "usage"; usage: LanguageModelUsage } | { kind: "cache-hit" }) or a dedicatedformatCacheHitResultLog()helper would avoid the stringly-typed branch and make the API surface clearer, though the current approach works correctly.🤖 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 `@src/llm/client.ts` around lines 609 - 651, The request/result logging helpers use a string sentinel mixed into the `LanguageModelUsage` type, which makes the API less type-safe. Update `formatRequestResultLog` and `formatRequestUsageLog` to use a discriminated result shape or split out a dedicated `formatCacheHitResultLog` helper, and adjust the `"cache-hit"` branch accordingly while keeping `formatRequestErrorStackLog` and `formatTokenCount` behavior unchanged.src/wikg/wikg-coordinator.ts (1)
1748-1774: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider an index on
mutation_tokenif this table grows.The lookup filters on
mutation_tokenwith no supporting index (only PK isarchive_key, entry_path), so this is a full-table scan. Likely fine at expectedentry_overlaysscale, but worth an index if the number of tracked archives/overlays grows significantly.🤖 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 `@src/wikg/wikg-coordinator.ts` around lines 1748 - 1774, The query in listSearchIndexAdoptionCandidates filters by mutation_token and entry_path while the existing primary key on entry_overlays does not support that lookup, so add an index that matches the WHERE clauses used here. Update the schema/migration for entry_overlays to include an index on mutation_token, ideally combined with entry_path and/or archive_key if that better fits the query pattern, and ensure any database setup code that defines the table stays consistent with the new index.src/topology/segment-incision.ts (1)
180-194: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUse binary search for segment containment lookups.
Line 186 scans every segment start for each chunk and external edge endpoint; this makes the incision pass scale with
segments × (chunks + edge visits). SincestartSentenceIndexesis already sorted, a binary search keeps the same behavior with much lower cost.⚡ Proposed lookup optimization
function findContainingSegmentStartIndex( startSentenceIndexes: readonly number[], sentenceIndex: number, ): number | undefined { - let containingSegmentStartIndex: number | undefined; - - for (const startSentenceIndex of startSentenceIndexes) { - if (startSentenceIndex > sentenceIndex) { - break; - } - - containingSegmentStartIndex = startSentenceIndex; - } - - return containingSegmentStartIndex; + let low = 0; + let high = startSentenceIndexes.length - 1; + let containingSegmentStartIndex: number | undefined; + + while (low <= high) { + const mid = Math.floor((low + high) / 2); + const startSentenceIndex = startSentenceIndexes[mid]; + + if (startSentenceIndex === undefined) { + break; + } + if (startSentenceIndex > sentenceIndex) { + high = mid - 1; + continue; + } + + containingSegmentStartIndex = startSentenceIndex; + low = mid + 1; + } + + return containingSegmentStartIndex; }🤖 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 `@src/topology/segment-incision.ts` around lines 180 - 194, The containment lookup in findContainingSegmentStartIndex is doing a linear scan over already-sorted startSentenceIndexes for every sentenceIndex, which makes segment incision scale poorly. Replace the loop with a binary search that returns the greatest startSentenceIndex less than or equal to sentenceIndex, preserving the current undefined behavior when no segment starts before the target. Keep the change localized to findContainingSegmentStartIndex so the callers in the incision pass continue to work unchanged.
🤖 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 `@src/document/stores.ts`:
- Around line 817-824: The persistent implementation of
listBySentenceStartIndexes is only querying a single sentence index, which makes
it diverge from the snapshot store’s full-fragment behavior. Update
listBySentenceStartIndexes to expand each selected start through the full
segment range, using listBySentenceRange consistently so the DB-backed path
returns the same chunk boundaries as the snapshot store. Keep the fix localized
around listBySentenceStartIndexes and its call to listBySentenceRange.
In `@src/facade/build-queue.ts`:
- Around line 280-283: Validate the explicit jobId before it is passed into
createJobWorkspacePath, createJobCachePath, and createJobLogPath in
build-queue.ts, because options.jobId is used directly in filesystem paths and
later removed recursively. Add a guard in the job setup flow near the jobId
assignment to reject any provided ID containing path separators or dot segments
like . and .., while still allowing randomUUID-generated IDs to proceed
unchanged.
In `@src/topology/resource-segmentation.ts`:
- Around line 30-35: The sentence range calculation in resource-segmentation is
using the last segment start instead of the full segment boundary, which can
truncate multi-sentence groups. Update the grouping logic in SentenceGroupRecord
creation so the endSentenceIndex comes from the segment’s true end boundary,
either by carrying that boundary through SegmentInfo or computing it before
mapping groups in the resource-segmentation flow. Make sure the fix is applied
in the code path that builds SentenceGroupRecord objects from groups.
In `@src/wikg/archive.ts`:
- Around line 26-36: `validateArchiveMutationToken()` now hard-rejects archives
whose first entry is not `.wikg-mutation-token`, which breaks older token-less
archives without any compatibility path. Update the archive format handling in
`src/wikg/archive.ts` by either bumping `WIKG_FORMAT_VERSION` and treating this
as a deliberate format change, or adding a migration/backfill path in
`validateArchiveMutationToken()`/related archive-reading logic so pre-token
`.wikg` archives remain readable.
In `@src/wikg/wikg-coordinator.ts`:
- Around line 883-895: The search-index fallback in wikg-coordinator’s
archive-read path should tolerate missing mutation tokens in legacy or external
archives. In the block that calls tryAdoptSearchIndexCacheOverlay and then
re-reads the overlay, wrap the awaited adoption helper in a catch that ignores
adoption failures so readWikgArchiveMutationToken cannot abort resolution for
otherwise readable archives; keep the fallback behavior gated by
SEARCH_INDEX_DATABASE_ENTRY_PATH and the overlay file-kind check.
---
Outside diff comments:
In `@src/facade/build-queue.ts`:
- Around line 199-211: The build queue schema update for build_jobs is
incomplete because CREATE TABLE IF NOT EXISTS will not add the new cache_path
and log_path columns to existing databases. Update the build-queue
initialization/migration logic around BUILD_QUEUE_SCHEMA_SQL and mapBuildJob to
detect older queue DBs, run an ALTER TABLE/backfill step for build_jobs, and
ensure existing rows get valid values before they are read.
In `@src/wikg/wikg-coordinator.ts`:
- Around line 597-627: The dry-run path in the GC flow is skipping
descendant-directory accounting, so the report undercounts scanned, removed, and
freedBytes. Update the archive cleanup logic in wikg-coordinator’s workspace
scan to always call removeDisposableDescendantDirectories for counting purposes,
and only gate the actual destructive deletion behavior on context.dryRun,
matching the file-removal loop’s dry-run handling.
---
Nitpick comments:
In `@src/llm/client.ts`:
- Around line 365-376: The failure paths in `src/llm/client.ts` are logging the
same error twice: once through the standalone `[[Error]]` append and again
inside `formatRequestResultLog(tokenUsage, error)` via `[[Error Stack]]`. Update
the payment-required and non-retryable branches to keep only one error
representation, using either the direct `formatError(...)` append or the
`formatRequestResultLog` output, and apply the same cleanup to the other
matching block referenced in the comment.
- Around line 609-651: The request/result logging helpers use a string sentinel
mixed into the `LanguageModelUsage` type, which makes the API less type-safe.
Update `formatRequestResultLog` and `formatRequestUsageLog` to use a
discriminated result shape or split out a dedicated `formatCacheHitResultLog`
helper, and adjust the `"cache-hit"` branch accordingly while keeping
`formatRequestErrorStackLog` and `formatTokenCount` behavior unchanged.
In `@src/topology/segment-incision.ts`:
- Around line 180-194: The containment lookup in findContainingSegmentStartIndex
is doing a linear scan over already-sorted startSentenceIndexes for every
sentenceIndex, which makes segment incision scale poorly. Replace the loop with
a binary search that returns the greatest startSentenceIndex less than or equal
to sentenceIndex, preserving the current undefined behavior when no segment
starts before the target. Keep the change localized to
findContainingSegmentStartIndex so the callers in the incision pass continue to
work unchanged.
In `@src/wikg/wikg-coordinator.ts`:
- Around line 1748-1774: The query in listSearchIndexAdoptionCandidates filters
by mutation_token and entry_path while the existing primary key on
entry_overlays does not support that lookup, so add an index that matches the
WHERE clauses used here. Update the schema/migration for entry_overlays to
include an index on mutation_token, ideally combined with entry_path and/or
archive_key if that better fits the query pattern, and ensure any database setup
code that defines the table stays consistent with the new index.
In `@test/topology/grouping.test.ts`:
- Around line 61-86: The createSerialFragments test fixture is duplicated across
topology test files, so update the grouping.test.ts helper to use a shared
reusable test utility instead of maintaining a near-identical local copy.
Extract the common fixture logic from createSerialFragments into a shared
topology test helper that can be imported by grouping.test.ts,
segment-incision.test.ts, and the similar inline setup in topology.test.ts,
keeping the same ReadonlySerialFragments shape and behavior.
🪄 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: 3ab5cddb-26d1-4348-9303-cd268016aa82
📒 Files selected for processing (31)
data/help/commands/gc.jinjadata/help/topics/runtime.jinjasrc/archive/query/archive-view.tssrc/cli/progress-output.tssrc/cli/queue.tssrc/cli/stage-runtime.tssrc/common/logging.tssrc/document/stores.tssrc/facade/build-queue.tssrc/facade/chapter-build.tssrc/gc/files.tssrc/llm/client.tssrc/topology/grouping.tssrc/topology/resource-segmentation.tssrc/topology/segment-incision.tssrc/topology/topology.tssrc/wikg/archive.tssrc/wikg/wikg-coordinator.tstest/cli/queue.test.tstest/common/logging.test.tstest/document/stores.test.tstest/editor/markup.test.tstest/facade/build-queue.test.tstest/gc/gc.test.tstest/llm/client.test.tstest/topology/grouping.test.tstest/topology/resource-segmentation.test.tstest/topology/segment-incision.test.tstest/topology/topology.test.tstest/wikg/archive.test.tstest/wikg/spine-digest-file.test.ts
| const results = await Promise.all( | ||
| fragmentIds.map( | ||
| async (fragmentId) => | ||
| await this.listBySentenceRange(serialId, fragmentId, fragmentId), | ||
| sentenceStartIndexes.map( | ||
| async (sentenceStartIndex) => | ||
| await this.listBySentenceRange( | ||
| serialId, | ||
| sentenceStartIndex, | ||
| sentenceStartIndex, | ||
| ), |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify all callers and implementations agree on range semantics.
rg -n -C3 '\blistBySentenceStartIndexes\s*\(' src testRepository: oomol-lab/wiki-graph
Length of output: 2473
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the interface, persistent implementation, snapshot implementation, and tests
sed -n '1,120p' src/document/stores.ts
printf '\n--- PERSISTENT IMPLEMENTATION ---\n'
sed -n '780,860p' src/document/stores.ts
printf '\n--- SNAPSHOT IMPLEMENTATION ---\n'
sed -n '930,990p' src/facade/chapter-build.ts
printf '\n--- TESTS ---\n'
sed -n '280,340p' test/document/stores.test.tsRepository: oomol-lab/wiki-graph
Length of output: 9602
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect chunk/sentence model and the segment-range helper.
rg -n -C4 'createSegmentRanges|sentenceIds|sentenceId|sentence_index|listBySentenceStartIndexes' src testRepository: oomol-lab/wiki-graph
Length of output: 50377
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the segment-range helper and any tests covering snapshot semantics.
rg -n -C4 'function createSegmentRanges|createSegmentRanges\(|listBySentenceStartIndexes\(|segmentRanges' src testRepository: oomol-lab/wiki-graph
Length of output: 4452
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the segment-range helper definition.
sed -n '1372,1428p' src/facade/chapter-build.tsRepository: oomol-lab/wiki-graph
Length of output: 1554
Make listBySentenceStartIndexes return the whole fragment range in the persistent store. The DB-backed implementation only matches sentence_index = start, while the snapshot store expands each selected start to its full segment range. That can return different chunks for the same selection; align the persistent path with the same range semantics.
🤖 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 `@src/document/stores.ts` around lines 817 - 824, The persistent implementation
of listBySentenceStartIndexes is only querying a single sentence index, which
makes it diverge from the snapshot store’s full-fragment behavior. Update
listBySentenceStartIndexes to expand each selected start through the full
segment range, using listBySentenceRange consistently so the DB-backed path
returns the same chunk boundaries as the snapshot store. Keep the fix localized
around listBySentenceStartIndexes and its call to listBySentenceRange.
| const jobId = options.jobId ?? randomUUID(); | ||
| const workspacePath = await createJobWorkspacePath(archiveKey, jobId); | ||
| const workspacePath = await createJobWorkspacePath(jobId); | ||
| const cachePath = await createJobCachePath(jobId); | ||
| const logPath = await createJobLogPath(jobId); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== build-queue outline ==\n'
ast-grep outline src/facade/build-queue.ts --view expanded || true
printf '\n== jobId-related search ==\n'
rg -n "jobId|createJobWorkspacePath|createJobCachePath|createJobLogPath|rm\\(|remove|recursive" src/facade/build-queue.ts src -g '!**/dist/**' -g '!**/build/**' || trueRepository: oomol-lab/wiki-graph
Length of output: 47197
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1490,1545p' src/facade/build-queue.ts
printf '\n== rm call sites around job cleanup ==\n'
sed -n '920,990p' src/facade/build-queue.ts
printf '\n== disposable child dir helper ==\n'
sed -n '1,220p' src/gc/files.tsRepository: oomol-lab/wiki-graph
Length of output: 8172
Validate jobId before using it in filesystem paths. options.jobId is joined directly into workspace/cache/log/events paths, and those paths are later removed recursively; reject separators and ./.. for explicit IDs to prevent path traversal through this exported API.
🤖 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 `@src/facade/build-queue.ts` around lines 280 - 283, Validate the explicit
jobId before it is passed into createJobWorkspacePath, createJobCachePath, and
createJobLogPath in build-queue.ts, because options.jobId is used directly in
filesystem paths and later removed recursively. Add a guard in the job setup
flow near the jobId assignment to reject any provided ID containing path
separators or dot segments like . and .., while still allowing
randomUUID-generated IDs to proceed unchanged.
| return groups.map((startSentenceIndexes, groupId) => ({ | ||
| endSentenceIndex: Math.max(...startSentenceIndexes), | ||
| groupId, | ||
| serialId: input.serialId, | ||
| startSentenceIndex: Math.min(...fragmentIds), | ||
| startSentenceIndex: Math.min(...startSentenceIndexes), | ||
| })); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== file list ==\n'
git ls-files 'src/topology/*' | sed -n '1,200p'
printf '\n== outline: src/topology/resource-segmentation.ts ==\n'
ast-grep outline src/topology/resource-segmentation.ts --view expanded || true
printf '\n== surrounding source (numbered) ==\n'
cat -n src/topology/resource-segmentation.ts | sed -n '1,220p'
printf '\n== search for related types/usages ==\n'
rg -n "SentenceGroupRecord|SegmentInfo|createSegmentGroups|startSentenceIndex|endSentenceIndex|sentence range|segment boundary" src/topology src -g '!**/node_modules/**' || trueRepository: oomol-lab/wiki-graph
Length of output: 31523
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== segment-incision.ts ==\n'
cat -n src/topology/segment-incision.ts | sed -n '1,260p'
printf '\n== grouping.ts ==\n'
cat -n src/topology/grouping.ts | sed -n '1,120p'
printf '\n== SentenceGroupRecord definition ==\n'
cat -n src/document/types.ts | sed -n '135,155p'
printf '\n== downstream group consumers ==\n'
sed -n '250,330p' src/editor/editor.ts
printf '\n---\n'
sed -n '1150,1425p' src/facade/chapter-build.tsRepository: oomol-lab/wiki-graph
Length of output: 20531
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== listGroupSentences body ==\n'
sed -n '330,390p' src/facade/chapter-build.ts
printf '\n== any direct range clipping against SentenceGroupRecord ==\n'
rg -n "group\.endSentenceIndex|group\.startSentenceIndex|SentenceGroupRecord" src/facade/chapter-build.ts src/editor/editor.ts src/topology -A3 -B3Repository: oomol-lab/wiki-graph
Length of output: 8273
Use the last segment’s sentence boundary here. Math.max(...startSentenceIndexes) only returns the final segment start, so multi-sentence segments get truncated and downstream sentence-range lookups can miss trailing sentences. Carry the segment end boundary through SegmentInfo or derive it before emitting SentenceGroupRecord. src/topology/resource-segmentation.ts:30-35
🤖 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 `@src/topology/resource-segmentation.ts` around lines 30 - 35, The sentence
range calculation in resource-segmentation is using the last segment start
instead of the full segment boundary, which can truncate multi-sentence groups.
Update the grouping logic in SentenceGroupRecord creation so the
endSentenceIndex comes from the segment’s true end boundary, either by carrying
that boundary through SegmentInfo or computing it before mapping groups in the
resource-segmentation flow. Make sure the fix is applied in the code path that
builds SentenceGroupRecord objects from groups.
| const WIKG_MUTATION_TOKEN_PATH = ".wikg-mutation-token"; | ||
| const WIKG_MANIFEST_PATH = "manifest.json"; | ||
| const SEARCH_INDEX_DATABASE_PATH = "fts.db"; | ||
| const WIKG_MANIFEST_CONTENT = `${JSON.stringify({ | ||
| formatVersion: WIKG_FORMAT_VERSION, | ||
| })}\n`; | ||
| const WIKG_MUTATION_TOKEN_MAGIC = "wikg-mutation-token:v1"; | ||
| const WIKG_MUTATION_TOKEN_PATTERN = /^[A-Za-z0-9_-]{43}$/u; | ||
|
|
||
| const WIKG_ARCHIVE_PATTERNS = [ | ||
| /^\.wikg-mutation-token$/u, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Look for any migration/backfill logic tied to mutation tokens or format version bumps.
rg -n "WIKG_FORMAT_VERSION|migrat|backfill|legacy" src/wikg -C3
rg -n "readWikgArchiveMutationToken|validateArchiveMutationToken" src -C3Repository: oomol-lab/wiki-graph
Length of output: 4540
🏁 Script executed:
#!/bin/bash
sed -n '1,220p' src/wikg/archive.ts
printf '\n---\n'
sed -n '220,620p' src/wikg/archive.ts
printf '\n---\n'
sed -n '1580,1675p' src/wikg/wikg-coordinator.tsRepository: oomol-lab/wiki-graph
Length of output: 18628
🏁 Script executed:
#!/bin/bash
rg -n "Unsupported WIKG format version|Missing WIKG mutation token|formatVersion|mutation token" src test tests -C3Repository: oomol-lab/wiki-graph
Length of output: 8071
Add a version gate or migration for token-less archives.
validateArchiveMutationToken() now rejects any archive whose first entry is not .wikg-mutation-token, but WIKG_FORMAT_VERSION stays at 1 and there’s no fallback for older archives. That makes pre-token .wikg files unreadable on upgrade; bump the format version or add a migration/backfill path so the compatibility break is explicit.
🤖 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 `@src/wikg/archive.ts` around lines 26 - 36, `validateArchiveMutationToken()`
now hard-rejects archives whose first entry is not `.wikg-mutation-token`, which
breaks older token-less archives without any compatibility path. Update the
archive format handling in `src/wikg/archive.ts` by either bumping
`WIKG_FORMAT_VERSION` and treating this as a deliberate format change, or adding
a migration/backfill path in `validateArchiveMutationToken()`/related
archive-reading logic so pre-token `.wikg` archives remain readable.
| let overlay = await this.#readOverlay(entryPath); | ||
|
|
||
| if ( | ||
| entryPath === SEARCH_INDEX_DATABASE_ENTRY_PATH && | ||
| overlay?.kind !== "file" | ||
| ) { | ||
| await tryAdoptSearchIndexCacheOverlay({ | ||
| targetArchiveKey: this.#archiveKey, | ||
| targetArchivePath: this.#archivePath, | ||
| }); | ||
| overlay = await this.#readOverlay(entryPath); | ||
| } | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
rg -n "mutation token|readWikgArchiveMutationToken|writeWikgArchiveMutationToken|WIKG mutation token|archive" src/wikg/archive.ts src/wikg -g '!**/dist/**'Repository: oomol-lab/wiki-graph
Length of output: 27607
🏁 Script executed:
sed -n '150,260p' src/wikg/archive.tsRepository: oomol-lab/wiki-graph
Length of output: 2826
🏁 Script executed:
sed -n '260,420p' src/wikg/archive.tsRepository: oomol-lab/wiki-graph
Length of output: 4348
🏁 Script executed:
sed -n '1,150p' src/wikg/archive.tsRepository: oomol-lab/wiki-graph
Length of output: 4245
🏁 Script executed:
rg -n "WIKG_FORMAT_VERSION|formatVersion|mutation token|readWikgArchiveMutationToken|createWikgMutationTokenContent|writeWikgArchiveWithOverlays|extractWikgArchive" srcRepository: oomol-lab/wiki-graph
Length of output: 2133
Guard the token-adoption fallback
writeWikgArchive* always adds .wikg-mutation-token, but readWikgArchiveMutationToken still throws when that entry is missing. Legacy or externally produced archives can legitimately lack the token, so this awaited helper can still abort search-index resolution even though the archive is otherwise readable. Catch and ignore adoption failures here.
🤖 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 `@src/wikg/wikg-coordinator.ts` around lines 883 - 895, The search-index
fallback in wikg-coordinator’s archive-read path should tolerate missing
mutation tokens in legacy or external archives. In the block that calls
tryAdoptSearchIndexCacheOverlay and then re-reads the overlay, wrap the awaited
adoption helper in a catch that ignores adoption failures so
readWikgArchiveMutationToken cannot abort resolution for otherwise readable
archives; keep the fallback behavior gated by SEARCH_INDEX_DATABASE_ENTRY_PATH
and the overlay file-kind check.
1 participant
Summary
Tests