Fix source text rendering and graph build locking#105
Conversation
Summary by CodeRabbit
WalkthroughChangesThis PR adds README screenshot images and reworks two independent areas: knowledge-graph artifact generation now uses a two-step snapshot-then-generate flow (new Sequence Diagram(s)sequenceDiagram
participant CLI as queue.ts
participant Facade as knowledge-graph-build.ts
participant Doc as ArchiveDocument
CLI->>Doc: assertCurrentBuildInputRevision
CLI->>Facade: snapshotChapterKnowledgeGraphInput(document, chapterId)
Facade->>Doc: readChapterFragments / read chapter details
Facade-->>CLI: ChapterKnowledgeGraphInputSnapshot
CLI->>Facade: generateChapterKnowledgeGraphArtifactFromSnapshot(chapterId, snapshot, options)
Facade->>Facade: validate snapshot.chapterId and stage
Facade-->>CLI: ChapterKnowledgeGraphBuildArtifact
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/archive/query/archive-view.ts (1)
4137-4178: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
joinTextStreamSentencesmerges words at sentence boundaries.
joinTextStreamSentences(used as the fallback in bothreadTextStreamRangeandreadTextStreamText) joins already-normalizedsentence.textvalues with"".Sentence.textis fully trimmed of leading/trailing whitespace (pernormalizeSentenceText), so any natural space that originally separated two sentences is lost when concatenating with an empty separator — for real segmented text this merges the last word of one sentence with the first word of the next (e.g. the updated test fixtures show"NoteAlice","retrieval.Bob","agents.朱元璋").This directly undercuts the PR's goal of producing clean, non-garbled evidence text for relation-discovery prompts — merged words could confuse downstream extraction just as much as the artificial newlines this PR set out to remove.
🩹 Suggested fix
function joinTextStreamSentences( sentences: readonly Pick<ArchiveTextStreamSentence, "text">[], ): string { - return sentences.map((sentence) => sentence.text).join(""); + return sentences.map((sentence) => sentence.text).join(" "); }Note: this will require updating the corresponding expected values in
test/archive/query/archive-view.test.ts(e.g. lines 304, 823, 1238, 1813, 2855, 3015) to include the separating space.Also applies to: 4180-4230
🤖 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/archive/query/archive-view.ts` around lines 4137 - 4178, `readTextStreamRange` is falling back to `joinTextStreamSentences`, which concatenates trimmed sentence text without a separator and merges words at sentence boundaries. Update the fallback (and the matching `readTextStreamText` path it also applies to) so sentence text is joined with a space or equivalent boundary-preserving separator, using the existing helpers like `joinTextStreamSentences` and `normalizeRenderedTextStreamRange` as the main entry points. After changing the join behavior, update the affected archive-view test expectations to include the inserted space.
🤖 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 `@README_zh-CN.md`:
- Line 1: The Markdown image in the README lacks alternative text, which hurts
accessibility and may trigger markdownlint. Update the image syntax to include a
short, clear Chinese alt description, and locate it by the standalone image
reference in the README content.
In `@README.md`:
- Line 1: The README image is missing alternate text, which affects
accessibility and markdownlint compliance. Update the Markdown image in README
to include a short descriptive alt label so the image can be identified by
screen readers; locate it by the terminal screenshot reference currently using
the image syntax in README.
In `@src/document/text-streams.ts`:
- Around line 734-748: The span calculation in the sentence-building loop is
using segment.text.length, which can drift if the segmenter normalizes or trims
text. Update the raw-source range logic in the segment iteration (the Sentence
creation and byteOffset/byteLength assignment) to derive the end of each span
from the next segment’s offset or a raw length provided by the segmenter, so the
stored offsets always map to the original text and readTextInRange reconstructs
correctly.
---
Outside diff comments:
In `@src/archive/query/archive-view.ts`:
- Around line 4137-4178: `readTextStreamRange` is falling back to
`joinTextStreamSentences`, which concatenates trimmed sentence text without a
separator and merges words at sentence boundaries. Update the fallback (and the
matching `readTextStreamText` path it also applies to) so sentence text is
joined with a space or equivalent boundary-preserving separator, using the
existing helpers like `joinTextStreamSentences` and
`normalizeRenderedTextStreamRange` as the main entry points. After changing the
join behavior, update the affected archive-view test expectations to include the
inserted space.
🪄 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: 3672d9b8-546f-4c88-8440-6eca4a6d4edb
⛔ Files ignored due to path filters (4)
docs/images/app-screenshot-cn.pngis excluded by!**/*.pngdocs/images/app-screenshot-en.pngis excluded by!**/*.pngdocs/images/terminal-cn.pngis excluded by!**/*.pngdocs/images/terminal-en.pngis excluded by!**/*.png
📒 Files selected for processing (13)
README.mdREADME_zh-CN.mdsrc/archive/query/archive-view.tssrc/cli/queue.tssrc/document/text-streams.tssrc/document/types.tssrc/facade/index.tssrc/facade/knowledge-graph-build.tssrc/serial.tstest/archive/query/archive-view.test.tstest/cli/queue.test.tstest/facade/knowledge-graph-build.test.tstest/serial.test.ts
| @@ -1,3 +1,5 @@ | |||
|  | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
补上图片替代文本。
当前 Markdown 图片没有 alt 文本，会影响可访问性，也会触发 markdownlint。建议加一个简短、明确的中文说明。
♻️ 建议修改
-
+📝 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.
|  | |
|  |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 1-1: Images should have alternate text (alt text)
(MD045, no-alt-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 `@README_zh-CN.md` at line 1, The Markdown image in the README lacks
alternative text, which hurts accessibility and may trigger markdownlint. Update
the image syntax to include a short, clear Chinese alt description, and locate
it by the standalone image reference in the README content.
Source: Linters/SAST tools
| @@ -1,3 +1,5 @@ | |||
|  | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add alt text to the README image.
The current Markdown image has no alternate text, which hurts accessibility and trips markdownlint. Use a short descriptive label.
♻️ Proposed fix
-
+📝 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.
|  | |
|  |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 1-1: Images should have alternate text (alt text)
(MD045, no-alt-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 `@README.md` at line 1, The README image is missing alternate text, which
affects accessibility and markdownlint compliance. Update the Markdown image in
README to include a short descriptive alt label so the image can be identified
by screen readers; locate it by the terminal screenshot reference currently
using the image syntax in README.
Source: Linters/SAST tools
| for await (const segment of segmenter.pipe([text])) { | ||
| const rawText = text.slice( | ||
| segment.offset, | ||
| segment.offset + segment.text.length, | ||
| ); | ||
|
|
||
| if (sentence === "") { | ||
| if (rawText.trim() === "") { | ||
| continue; | ||
| } | ||
| const sentence = new Sentence(rawText, segment.wordsCount); | ||
|
|
||
| spans.push({ | ||
| byteLength: Buffer.byteLength(sentence, "utf8"), | ||
| byteOffset: Buffer.byteLength(text.slice(0, sentenceStart), "utf8"), | ||
| text: sentence, | ||
| wordsCount: countWords(sentence), | ||
| Object.assign(sentence, { | ||
| byteLength: Buffer.byteLength(rawText, "utf8"), | ||
| byteOffset: Buffer.byteLength(text.slice(0, segment.offset), "utf8"), | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find TextStreamSentenceSegmenter implementations / pipe() providers passed to writeTextStream
rg -nP -C4 '\bpipe\s*\(' src --type=ts -g '!**/*.test.ts'
echo '--- segmenter option call sites ---'
rg -nP -C4 'writeTextStream\s*\(' src --type=ts
echo '--- segmenter objects ---'
rg -nP -C6 'segmenter\s*[:=]' src --type=ts -g '!**/*.test.ts'Repository: oomol-lab/wiki-graph
Length of output: 21545
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- text-streams contract and usage ---'
sed -n '1,120p' src/document/text-streams.ts
echo '--- splitTextIntoCustomSentenceSpans ---'
sed -n '716,760p' src/document/text-streams.ts
echo '--- reader segment adapter interface ---'
sed -n '1,120p' src/reader/segment/types.ts
echo '--- default adapter implementation ---'
sed -n '1,220p' src/reader/segment/intl-segmenter.ts
echo '--- adapter entrypoint ---'
sed -n '1,80p' src/reader/segment/segment.ts
echo '--- look for any other TextStreamSentenceSegmenter implementations ---'
rg -n "implements TextStreamSentenceSegmenter|pipe\\(stream:.*AsyncIterable|segment\\.text|segment\\.offset" src -g '!**/*.test.ts'Repository: oomol-lab/wiki-graph
Length of output: 9451
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- splitTextIntoSentenceSpans and related helpers ---'
sed -n '640,720p' src/document/text-streams.ts
echo '--- sentence record usage / readTextInRange ---'
rg -n -C3 'readTextInRange|getSentence\\(|byteOffset|byteLength|sentenceIndex' src/document/text-streams.ts src/document -g '!**/*.test.ts'Repository: oomol-lab/wiki-graph
Length of output: 2164
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- writeTextStream option types ---'
sed -n '1,220p' src/document/text-streams.ts
echo '--- serial segmenter type definitions ---'
sed -n '1,220p' src/serial.ts
echo '--- reader segmenter type definitions ---'
sed -n '1,120p' src/reader/segment/types.ts
sed -n '1,120p' src/reader/reader.ts
echo '--- all segmenter-related public APIs ---'
rg -n -C2 'segmenter|SentenceStreamAdapter|TextStreamSentenceSegmenter|segmentTextStream|writeTextStream' src -g '!**/*.test.ts'Repository: oomol-lab/wiki-graph
Length of output: 33157
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- document/fragments.ts around writeTextStream ---'
sed -n '1,360p' src/document/fragments.ts
echo '--- document index exports ---'
sed -n '1,240p' src/document/index.ts
echo '--- any writeTextStream option/type re-exports ---'
rg -n -C2 'TextStreamWriteOptions|WriteTextStreamOptions|segmenter' src/document src/facade src/serial.ts -g '!**/*.test.ts'Repository: oomol-lab/wiki-graph
Length of output: 20940
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- location lookup and range reading ---'
rg -n -C4 'readTextInRange|getSentenceLocation|byteOffset|byteLength|sentenceIndex' src/document/text-streams.ts
echo '--- surrounding implementation ---'
sed -n '430,640p' src/document/text-streams.tsRepository: oomol-lab/wiki-graph
Length of output: 12406
Use a raw-source span here. segment.text.length only works if the segmenter returns the exact substring from text; if it trims or normalizes output, the stored byteOffset/byteLength will be wrong and readTextInRange will reconstruct the wrong content. Use the next segment’s offset or expose a raw length from the segmenter.
🤖 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/text-streams.ts` around lines 734 - 748, The span calculation in
the sentence-building loop is using segment.text.length, which can drift if the
segmenter normalizes or trims text. Update the raw-source range logic in the
segment iteration (the Sentence creation and byteOffset/byteLength assignment)
to derive the end of each span from the next segment’s offset or a raw length
provided by the segmenter, so the stored offsets always map to the original text
and readTextInRange reconstructs correctly.
1 participant
Summary
Verification