feat(files): line paging for vault_read_file text results - #397
Merged
Conversation
start_line/limit page any text rendition — passthrough formats, canvas outlines and raw JSON, PDF-extracted text — as a 1-based line window, preceded by a metadata block stating the window, total line count, and next start_line. A read without paging inputs stays byte-identical. The 100 KiB output cap now applies per window, so files past the cap are readable in pages; the cap's tool-description entry points at paging while runtime messages stay fact-only per the error-surface convention. A start line past the end errors with the total; images and raw PDF page rendering reject paging inputs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ST7bwmeMkH7ETvyytfNfJB
PR Code Suggestions ✨Explore these optional code suggestions:
|
![umm-actually[bot]](https://avatars.githubusercontent.com/in/4233716?s=60&v=4){kind=small}
Owner
Author
|
@CodeRabbit review |
✅ Action performedReview finished.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughChangesLine-range asset reading
Authoring guidance
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/vault-mcp/vault-operations/asset-operations.ts`:
- Around line 119-172: Update buildPagedTextResult to validate the paging inputs
before calling contentLines.slice: reject any startLine or limit value below 1
with an error instead of allowing negative indices or invalid windows. Preserve
the existing default of 1 for an omitted startLine and all valid paging
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9427d0ac-ce02-481f-8cec-c78d0b76674c
📒 Files selected for processing (7)
ARCHITECTURE.mdDOCKERHUB.mdREADME.mdsrc/vault-mcp/mcp-core/__tests__/tool-definitions.test.tssrc/vault-mcp/mcp-core/tools/asset-tools.tssrc/vault-mcp/vault-operations/__tests__/asset-operations.test.tssrc/vault-mcp/vault-operations/asset-operations.ts
CodeRabbit review: the tool schema's .min(1) made a sub-1 startLine unreachable, but a bypassed negative slice start would silently serve lines from the END of the rendition — the data layer enforces the bound itself per the repo's validation convention, with a fact-only message in the module's own naming. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ST7bwmeMkH7ETvyytfNfJB
The hasTrailingNewlineArtifact false path was untested — all existing paging tests used content ending in "\n". This test exercises paging a file without a trailing newline to prove the branch counts lines correctly. Mutation-verified: removing the conditional and always slicing off the last element fails this test. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
umm-actually re-reviewed at 1 new finding(s) posted (1 tracked finding(s) across all runs). umm-actually · deepseek/deepseek-v4-pro |
…NTS.md as write-time guidance Destructure lineWindow, name the end-of-file boolean, flatten the return. Reframe the code style preamble: these are authoring rules applied while writing, not a review checklist applied after. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Owner
Author
|
@umm review |
Owner
Author
|
@CodeRabbit review |
✅ Action performedReview finished.
|
Extract hasInvalidLineRange, isStartPastEnd, exceedsByteCap so each guard reads as intent, not arithmetic. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
metadataBlock and contentBlock make the two text blocks' roles visible without reading describeTextWindow. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
6 tasks
aliasunder
added a commit
that referenced
this pull request
Aug 4, 2026
## Summary - Mirrors PR #397's `vault_read_file` paging on `vault_read_note` — one paging idiom across both read tools - Pages the delivered rendition (full body or a `heading:` section) by 1-based line range; JSON modes (`outline`, `properties_only`) reject paging with a clear error - Fixes the oversized-section gap: this project's own Done lane holds 250+ cards — `heading: "Done", start_line: 1, limit: 20` now reads a window instead of the entire lane ### Implementation - **Extracted** `pageTextByLines` + `LineWindow` type to `obsidian-markdown/lines.ts` as a shared primitive (split, validate, slice — no byte cap; byte cap enforcement stays in asset-operations) - **Refactored** `buildPagedTextResult` in asset-operations.ts to delegate to `pageTextByLines` - **Moved** `describeTextWindow` to `tool-helpers.ts` for shared use by both read tools - **Added** `start_line`/`limit` schema params + paging validation + two-block result formatting to `vault_read_note` handler - **Updated** ARCHITECTURE.md input table and `vault_read_note` description (examples, errors, returns) ## Test plan - [x] 12 unit tests for `pageTextByLines` (window, clamp, CRLF, trailing newline, empty, past-EOF, invalid range) - [x] 9 integration tests for `vault_read_note` paging (full note, heading section, end-of-file, empty note, past-EOF error, outline/properties_only rejection, byte-identical unpaged, description assertions) - [x] 2342 tests pass (21 new, 2321 existing) - [x] Mutation-tested: removing paging from the section branch kills the heading-paging test - [x] Build + lint clean - [ ] CI green 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added optional line-based pagination for reading full notes and heading sections. * Paged responses include selected line ranges, total line counts, and continuation details. * Added validation for invalid, out-of-range, and incompatible pagination requests. * **Bug Fixes** * Improved handling of line endings, trailing newlines, empty content, and paging limits. * Preserved existing output formats for unpaged note reads. * **Documentation** * Updated tool documentation with pagination parameters, behavior, and limitations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
2 participants
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.
What does this PR do?
Large text files were unreadable: anything past the fixed 100 KiB text-output cap returned only
text output too large, with no way to read a slice. The cap's own comment has said since it landed that "text past this size needs paging" — this adds that paging.start_line+limitonvault_read_file. Two optional inputs page any text result — the seven passthrough formats, canvas outlines and raw JSON source, and PDF-extracted text — as a 1-based line window. A paged read prepends a metadata block so the agent always knows where it is and how much file exists:A read without paging inputs stays byte-identical to today's output (the existing verbatim
it.eachdoubles as the regression guard), so nothing changes for existing callers.start_line: 1, limit: 1doubles as a cheap "how many lines is this?" probe, documented in the tool description.Design decisions (user-ratified during planning):
start_lineover a 0-based offset — matchesvault_update_task'sline, editors, andwc -lthinking; the metadata block hands the agent the exact nextstart_line, so nobody does arithmetic.assertTextWithinCap, so paging is implemented at one interception point (buildPagedTextResult) replacing the three assert call sites — and the cap's documented remediation is true everywhere it fires. Images and PDFraw: truereject paging inputs.start line past the end: "data.csv" renders to 342 linescarries the self-correction datum but no wire input names or prescribed actions; all remediation ("page it with start_line and limit") lives in the tool description'sErrors:bullets. The unpaged cap message is unchanged, so its exact-match tests are untouched.splitIntoLines(the canonical CRLF normalizer) withwc -lcounting — a trailing newline's empty final element is not a line. Paged windows come back LF-joined with no trailing newline, a documented divergence from "exactly as written" that applies to paged reads only. The cap applies per window, so a single 100 KiB line still errors (…lines 1–1 render to 102401 bytes…). A start line past the end errors rather than returning an empty window an agent could misread as "file is empty"; an empty file returns the zero-line window.The text arm of
AssetReadResultnow carriespath(matching the image and pages arms) so the formatter can build the metadata line; 7 existing PDF whole-object assertions gained the field.Docs: ARCHITECTURE.md Files section (input row + text-formats item), README Files bullet (user-facing phrasing), DOCKERHUB.md regenerated. server.json, wiki.json, and env surfaces verified unaffected —
MAX_TEXT_OUTPUT_BYTESdeliberately stays a non-env constant.Validation. 18 new tests across both layers: 13 mocked data-layer units (window geometry, CRLF→LF, trailing-newline counting, empty file, past-EOF, per-window cap, PDF text, raw canvas, image + raw-PDF rejections) and 5 real-temp-vault wire-shape tests (both content blocks asserted whole, end-of-file window,
[Error]:exact messages) plus a description-contract test for the new Errors bullets. Mutation-tested against committed state: an off-by-one in the window slice fails 8 window tests, disabling the per-window cap fails exactly the cap test, disabling the past-EOF guard fails exactly the two past-EOF tests. Live verification on a Test Deploy to follow before merge.Type of change
Checklist
npm testpasses (the pre-existing sharp recompression timeout flaked in the dev container —fit-image-to-byte-budget.test.ts, fails identically on the unmodified tree, noted on PRs feat(cli): guided optional settings in init + configure subcommand #393/test(cli): consolidate scripted Prompts and DockerRunner stubs into a shared helper #394; passes in CI)npm run lintpasses (0 errors; changed files clean)npm run prettier:checkpassesnpm run buildsucceedsvault_read_filedescription keeps the Example / When to use / Returns / Errors structure🤖 Generated with Claude Code
https://claude.ai/code/session_01ST7bwmeMkH7ETvyytfNfJB
Summary by CodeRabbit
New Features
Bug Fixes
Documentation