From 150cf5c07104cd9e943eb76aca999d3e21f85d01 Mon Sep 17 00:00:00 2001 From: Aaron Bockelie Date: Tue, 19 May 2026 00:24:56 -0500 Subject: [PATCH] =?UTF-8?q?docs(adr):=20amend=20ADR-203=20=E2=80=94=20char?= =?UTF-8?q?-budget=20pagination=20with=20line=20bookends?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refined through design dialogue: - paginate/no-paginate boundary is a CHARACTER budget (READ_PAGE_CHARS, default 50000), not a line count (line count is an invalid proxy for context cost — 1500 short vs 1500 long lines differ by orders of magnitude) - bookends stay line-based (lineStart/lineEnd/totalLines) so edit.at_line on a large file still works; a page = longest whole-line run within the char budget - fits budget → whole file verbatim, one load (common case, no ceremony) - exceeds → bookended page 1 (single contiguous block, not chunk array) + nextPage hint; sequential paging via new `page` param - returnFullFile:true RETAINED and repurposed as the explicit whole-large-file override (not retired) - hard invariant recorded: a default read must never hand the agent a context-breaking raw dump Alternatives added: no-guard full dump, line-count budget, chunk-array — all rejected with rationale. --- ...t-reads-concise-by-default-tool-results.md | 91 ++++++++++++++----- 1 file changed, 69 insertions(+), 22 deletions(-) diff --git a/docs/architecture/tools/ADR-203-faithful-by-default-content-reads-concise-by-default-tool-results.md b/docs/architecture/tools/ADR-203-faithful-by-default-content-reads-concise-by-default-tool-results.md index 516c603..7f81110 100644 --- a/docs/architecture/tools/ADR-203-faithful-by-default-content-reads-concise-by-default-tool-results.md +++ b/docs/architecture/tools/ADR-203-faithful-by-default-content-reads-concise-by-default-tool-results.md @@ -66,14 +66,43 @@ from **action/result rendering** rather than by one blunt format switch. Adopt a **two-bucket response contract**. This ADR is the decision of record; implementation is tracked separately and is *not* part of this ADR. -1. **Content reads are faithful by default.** `vault.read` returns the - **complete, verbatim file source** by default — byte-exact as stored by - Obsidian (newlines, indentation, trailing whitespace, tabs, fenced - blocks, frontmatter delimiters preserved). Fragmentation / summarization - becomes an **explicit opt-in** for large-file context savings via the - *existing* knobs (`query`, `strategy`, `maxFragments`); `returnFullFile` - is no longer required to get the truth and is retired or demoted to a - no-op alias. The read→edit round-trip is correct by construction. +1. **Content reads are faithful by default — but never context-breaking.** + `vault.read` returns **byte-exact source** as stored by Obsidian + (newlines, indentation, trailing whitespace, tabs, fenced blocks, + frontmatter delimiters preserved — never flattened/fragmented unless + fragments are explicitly requested). The hard invariant: **a default read + must not hand the agent a context-breaking raw dump.** + + The paginate/no-paginate boundary is a **character budget** + (`READ_PAGE_CHARS`, default **50000** ≈ ~12k tokens, one tunable + constant) — *not* a line count. A line count is an invalid proxy for the + thing being bounded: 1500 five-word lines and 1500 hundred-word lines + have wildly different context cost. **Bookends stay line-based** (for + `edit.at_line`); the two reconcile by defining a *page* as the longest + run of **whole lines whose cumulative size ≤ `READ_PAGE_CHARS`**, + reported with the line range it covers. Lines are never split mid-line. + + - **Fits the budget (the common case — most notes):** the **whole file, + verbatim, in one load**. No pagination structure, no ceremony. + - **Exceeds the budget:** **page 1 only** — a single *contiguous* + verbatim block: the longest whole-line run within `READ_PAGE_CHARS` + (not an array of chunk objects), carrying **bookends**: `lineStart`, + `lineEnd`, `totalLines`, `bytes`, and a `nextPage` hint + (`vault.read(path, page=2)`). Absolute line numbers are preserved, so + `edit.at_line` on a large file still works precisely and the agent + pages forward instead of a truncated preview, lossy fragments, or a + context bomb. (A single line that alone exceeds the budget is returned + whole as its own page — never split — flagged; correctness wins over + the budget in that rare case.) + - **`returnFullFile: true`:** explicit override — the entire file + verbatim regardless of size. `returnFullFile` is therefore **retained + and repurposed** (not retired): it is the deliberate large-context + opt-in, used when the agent knowingly accepts the cost. + - **`query` / `strategy` / `maxFragments`:** semantic fragment retrieval, + unchanged. + + The read→edit round-trip is correct by construction in every branch + (per-page content is byte-exact for its line range). 2. **Action/result outputs stay concise-formatted by default.** Tools whose result is a status / confirmation / list (create, update, delete, move, @@ -94,12 +123,13 @@ record; implementation is tracked separately and is *not* part of this ADR. `_Formatter error_` fallback; the failing `returnFullFile` formatted branch is fixed or removed as part of (1). -Backward compatibility: (1) changes the default shape/size of `vault.read` -for clients that currently rely on the fragmented/flattened default. This -is a deliberate breaking change to fix a correctness bug; it ships with a -clear changelog entry and a documented opt-in (`strategy`/`maxFragments`) -for the previous context-saving behaviour. A prerelease + functional -verification of a real read→`edit.window` round-trip gates the rollout. +Backward compatibility: (1) changes the default shape of `vault.read` for +clients relying on the fragmented/flattened default. Deliberate breaking +change to fix a correctness bug; ships with a changelog entry, the +`strategy`/`maxFragments` opt-in for the old context-saving behaviour, and +`returnFullFile:true` for whole-large-file access. A prerelease + functional +verification of a real read→`edit.window` round-trip (incl. a large file +exercising bookended page 1 → `at_line`) gates the rollout. #133 is **reframed, not closed by fiat**: it remains the user-facing tracking issue for bucket 1; this ADR records the agreed direction and @@ -124,19 +154,24 @@ explicitly supersedes its specific "global `raw` flip" remedy. default `vault.read` (smaller, lossy payloads). Mitigated by changelog, the existing `strategy`/`maxFragments` opt-in for context savings, and a gated prerelease test. -- Large files now return full content by default → higher per-call context - cost for the read-everything pattern. Mitigated: fragmentation is still - one explicit parameter away, and a `wordCount`/size `warning` is retained - to nudge large-file callers toward it. +- Large files no longer return verbatim by default — they return a + bookended page 1. This is a behavioural change for any client that read a + large file in one default call; `returnFullFile:true` restores that + explicitly. Accepted: a context-breaking default is the failure this ADR + exists to prevent. +- Pagination adds a small surface to `vault.read` (`page` param, + `lineStart/lineEnd/totalLines/bytes/nextPage` fields). Justified: it's + what lets large-file `edit.at_line` keep working instead of forcing + fragments. ### Neutral - Two-bucket contract should be documented in the README and tool descriptions so the default semantics are discoverable. -- `returnFullFile` becomes redundant; retire or alias with a deprecation - note. -- No change to `IObsidianAPI` or the MCP tool surface beyond the default - response shape/size of `vault.read` and the de-duplicated envelope. +- `returnFullFile` is retained and repurposed as the explicit + whole-large-file override (not retired). +- No change to `IObsidianAPI`; the MCP `vault` tool gains a `page` + parameter and the read response gains pagination bookends. ## Alternatives Considered @@ -155,3 +190,15 @@ explicitly supersedes its specific "global `raw` flip" remedy. - **New dedicated `vault.source` tool, leave `read` lossy.** Rejected: splits the surface and leaves the obvious tool (`read`) a footgun; the principled fix is making the obvious tool correct by default. +- **Return full verbatim regardless of size (no guard).** Rejected + outright: a large file dumped raw breaks the agent's context — this is + the exact failure mode the pagination guard exists to prevent, and the + reason `returnFullFile:true` is an *explicit, knowing* opt-in. +- **Line-count page budget (e.g. 1500 lines).** Rejected: line count does + not bound context — 1500 short lines vs 1500 long lines differ by orders + of magnitude. The budget must be size-based (`READ_PAGE_CHARS`); only the + *bookends* are line-based, for `at_line`. +- **Array of page/chunk objects in one response.** Rejected: that is just + the fragmented default in another shape and is not "one load." A page is + a single contiguous verbatim block; multi-page access is sequential via + the `page` parameter.