From 95ca216074c3d526b98333f2aa44745bd1a32b9c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 09:00:33 +0000 Subject: [PATCH 1/2] A listing hands back the note's opening it promised (#6) `NoteSummary.excerpt` is documented as the text around a search match, or the note's opening otherwise. Only `search` ever filled it in. `list_notes` and `backlinks` both called `_summary(entry)` with no excerpt argument and got the "" default, so every note in a listing came back with an empty one and a client had to open each note to learn what it was about, which is the work the field exists to save. The opening is now derived in `index._parse_note`, which already has the note parsed, and `_summary` falls back to it when there is no match to quote. Doing it there rather than when a listing asks costs no extra read: on demand it would be one file open per note listed, and `backlinks` is not paginated, so the MOC note that a whole vault links to would open the whole vault. The filter-only search path gets the same excerpt off the index instead of re-reading every note on the page, which it used to do. The derivation moved to its own `excerpts.py`. `markdown.py` is where it would otherwise sit, next to the other `*_of(note)` scanners, and that file is 18 lines from the 500-line budget. Held on the same terms as the headings and properties beside it: bounded, derived, and dropped when the note's mtime moves. A test covers that, since a cached excerpt outliving the note it was cut from would read as current and not be. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CrfMhG31RFg4eY1sMnS4dr --- CLAUDE.md | 7 +- knap_mcp/providers/filesystem/excerpts.py | 49 ++++++++++++++ knap_mcp/providers/filesystem/index.py | 16 ++++- knap_mcp/providers/filesystem/provider.py | 10 ++- knap_mcp/providers/filesystem/search.py | 28 ++------ tests/test_provider.py | 78 +++++++++++++++++++++++ 6 files changed, 159 insertions(+), 29 deletions(-) create mode 100644 knap_mcp/providers/filesystem/excerpts.py diff --git a/CLAUDE.md b/CLAUDE.md index c180e44..05c6c48 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -84,8 +84,9 @@ package. `paths.py` (confinement), `markdown.py` (scanning: links, headings, tags, atomic write, revs), `frontmatter.py` (writing properties without rewriting the block), `index.py` (lazy link/tag/property index, invalidated on mtime, never - rebuilt per call), `search.py`, `periodic.py` (daily notes from the vault's own - settings), `writes.py` (the write half, as a mixin), `provider.py`. + rebuilt per call), `search.py`, `excerpts.py` (the note opening a listing shows + when there is no match to quote), `periodic.py` (daily notes from the vault's + own settings), `writes.py` (the write half, as a mixin), `provider.py`. - **The index holds hidden notes and the lookups exclude them.** Indexing with `include_hidden=False` meant `include_hidden=True` on a search had nothing to find, because the notes were never there. So the walk takes everything, and @@ -209,7 +210,7 @@ make test-all | `__main__.py` | CLI entry: argparse, transport selection | | `config.py` | `KnapConfig` dataclass + env loading | | `providers/protocol.py` | `VaultProvider` protocol + value objects | -| `providers/filesystem/` | The backend: paths, markdown, frontmatter, index, search, periodic, writes | +| `providers/filesystem/` | The backend: paths, markdown, frontmatter, index, search, excerpts, periodic, writes | | `providers/factory.py` | Backend selection from config | | `tools/handler.py` | `VaultToolHandler` (mixins) + `register_tools` | | `tools/vault/` | Tools as mixins: browse, query, read, write, organize, graph, periodic, attachments | diff --git a/knap_mcp/providers/filesystem/excerpts.py b/knap_mcp/providers/filesystem/excerpts.py new file mode 100644 index 0000000..8ea1feb --- /dev/null +++ b/knap_mcp/providers/filesystem/excerpts.py @@ -0,0 +1,49 @@ +"""The opening of a note: what a listing shows when there is no match to quote. + +A search hit can quote the text around the query. A listing has no query, so the +only honest thing to show is how the note starts, and both `protocol.NoteSummary` +and `schemas.NoteSummaryResult` promise exactly that. + +It sits in its own module because the two callers that need it are on opposite +sides of the backend and neither owns it: `index.py` derives it while it has the +note parsed anyway, and `search.py` hands it back for a filter-only query. The +obvious third home, `markdown.py` next to the other `*_of(note)` scanners, is 18 +lines from the 500-line budget, and this is not the change that should spend them. +""" + +from __future__ import annotations + +import re + +from . import markdown as md + +#: How much of the opening to keep. Long enough to tell two notes apart, short +#: enough that a page of fifty of them is not a wall of text. +OPENING_CHARS = 160 + +#: Lines of prose to draw the opening from. More than a couple, because the first +#: line of a note is often a single short sentence; few enough that the cost is a +#: slice rather than a scan of a long note. +OPENING_LINES = 4 + + +def opening_of(note: md.ParsedNote, *, chars: int = OPENING_CHARS) -> str: + """The first prose of a note, flattened to one line. + + The frontmatter is already off ``body``. Leading headings go too, because + "# Meeting notes" on a note titled "Meeting notes" tells a client nothing + that `title` did not already tell it, and a summary whose excerpt repeats its + own title is the same as having no excerpt. + + Whitespace is collapsed for the same reason it is collapsed around a search + match: this lands in a listing as one line, and a note that opens with an + indented list or a table should not arrive full of gaps. + """ + lines = note.body.strip().split("\n") + while lines and (not lines[0].strip() or lines[0].lstrip().startswith("#")): + lines.pop(0) + opening = re.sub(r"\s+", " ", " ".join(lines[:OPENING_LINES])).strip() + return opening[:chars] + ("..." if len(opening) > chars else "") + + +__all__ = ["OPENING_CHARS", "OPENING_LINES", "opening_of"] diff --git a/knap_mcp/providers/filesystem/index.py b/knap_mcp/providers/filesystem/index.py index eff420e..118afb4 100644 --- a/knap_mcp/providers/filesystem/index.py +++ b/knap_mcp/providers/filesystem/index.py @@ -23,13 +23,20 @@ from pathlib import Path from typing import Any, Dict, Iterable, List, Optional, Set, Tuple +from . import excerpts from . import markdown as md from . import paths as vault_paths @dataclass class NoteEntry: - """One note as the index knows it. No body: that is read on demand.""" + """One note as the index knows it. No body: that is read on demand. + + ``excerpt`` is not a body and not a step towards holding one. It is a couple + of lines, derived at parse time from a body that was in memory anyway, and + dropped the moment the note's mtime moves -- the same terms the headings and + the properties beside it are held on. + """ path: str rev: str @@ -42,6 +49,8 @@ class NoteEntry: properties: Dict[str, Any] = field(default_factory=dict) #: Link targets exactly as written in the note, in document order. link_targets: List[str] = field(default_factory=list) + #: The note's opening prose, for a listing that has no match to quote. + excerpt: str = "" @property def basename(self) -> str: @@ -177,6 +186,11 @@ def _parse_note(self, rel: str) -> Optional[NoteEntry]: headings=md.headings_of(note), properties=dict(note.frontmatter), link_targets=[link.target for link in md.raw_links_of(note)], + # Derived here rather than when a listing asks for it, because here + # the body is already in hand. Read on demand it would cost one file + # open per note listed, and `backlinks` is not paginated: the MOC that + # the whole vault links to would open the whole vault. + excerpt=excerpts.opening_of(note), ) def _rebuild_lookups(self) -> None: diff --git a/knap_mcp/providers/filesystem/provider.py b/knap_mcp/providers/filesystem/provider.py index 0f600a6..3cba038 100644 --- a/knap_mcp/providers/filesystem/provider.py +++ b/knap_mcp/providers/filesystem/provider.py @@ -336,6 +336,14 @@ def _compose(self, frontmatter_raw: str, body: str) -> str: return f"{frontmatter_raw}\n{body}" def _summary(self, entry: NoteEntry, *, excerpt: str = "") -> NoteSummary: + """One summary shape for a listing, a search hit and a backlink. + + ``excerpt`` is the text around a search match. Without one -- a listing, + a backlink list, a filter-only search -- the note's opening stands in, + which is what the protocol and the tool schema both promise. Defaulting + it to "" here was the whole of #6: three call sites, two of them silently + answering with nothing. + """ return NoteSummary( path=entry.path, title=entry.title, @@ -343,7 +351,7 @@ def _summary(self, entry: NoteEntry, *, excerpt: str = "") -> NoteSummary: size=entry.size, modified=_iso(entry.mtime_ns), tags=list(entry.tags), - excerpt=excerpt, + excerpt=excerpt or entry.excerpt, ) def _link_refs(self, rel: str, note: md.ParsedNote) -> List[LinkRef]: diff --git a/knap_mcp/providers/filesystem/search.py b/knap_mcp/providers/filesystem/search.py index 1af7dbb..9dfb315 100644 --- a/knap_mcp/providers/filesystem/search.py +++ b/knap_mcp/providers/filesystem/search.py @@ -92,10 +92,11 @@ def query( candidates.sort(key=lambda item: item.mtime_ns, reverse=True) if not needle: + # No query, so nothing to quote: the opening stands in, and it comes + # off the index rather than out of the file. This used to open every + # note on the page to read text the index had already parsed. page = candidates[offset : offset + limit] - return [Hit(entry=entry, excerpt=_opening(self.root, entry)) for entry in page], len( - candidates - ) + return [Hit(entry=entry, excerpt=entry.excerpt) for entry in page], len(candidates) hits: List[Hit] = [] scanned = 0 @@ -146,27 +147,6 @@ def _match(self, entry: NoteEntry, needle: str) -> Optional[str]: return None return _window(note.body, position, len(needle)) - def _opening(self, entry: NoteEntry) -> str: - return _opening(self.root, entry) - - -def _opening(root: Path, entry: NoteEntry, chars: int = 160) -> str: - """The first prose of a note, for a listing with no search term. - - Skips the frontmatter and the leading H1, because "# Meeting notes" under a - note called "Meeting notes" tells a client nothing it does not have. - """ - try: - text, _, _, _ = md.read_text(root / entry.path) - except OSError: - return "" - body = md.parse(text).body.strip() - lines = [line for line in body.split("\n")] - while lines and (not lines[0].strip() or lines[0].lstrip().startswith("#")): - lines.pop(0) - opening = " ".join(line.strip() for line in lines[:4]).strip() - return opening[:chars] + ("..." if len(opening) > chars else "") - def _window(body: str, position: int, length: int) -> str: start = max(0, position - EXCERPT_RADIUS) diff --git a/tests/test_provider.py b/tests/test_provider.py index 8a17970..91c18db 100644 --- a/tests/test_provider.py +++ b/tests/test_provider.py @@ -11,6 +11,7 @@ import pytest +from knap_mcp.providers.filesystem import excerpts from knap_mcp.providers.filesystem.provider import FilesystemVaultProvider from knap_mcp.providers.protocol import ProviderError @@ -266,6 +267,83 @@ def test_tags_are_most_used_first(self, provider) -> None: assert counts == sorted(counts, reverse=True) +class TestExcerpts: + """#6: the schema promises the opening for a listing, and got "" instead. + + `search` passed its match window through and the other two call sites did + not, so `vault_list_notes` and `vault_backlinks` answered with an empty + string on every note. A client that cannot see what a note is about from a + listing has to open all of them, which is the cost the excerpt exists to + avoid. + """ + + @staticmethod + def _excerpt_for(provider, path: str) -> str: + notes, _ = provider.list_notes(limit=100) + return next(note.excerpt for note in notes if note.path == path) + + def test_a_listing_carries_the_opening(self, provider) -> None: + excerpt = self._excerpt_for(provider, "Areas/Work/Acme.md") + assert excerpt.startswith("The renewal is due.") + + def test_every_note_in_a_listing_is_offered_one(self, provider) -> None: + """The bug was not partial: nothing in a listing had an excerpt.""" + notes, _ = provider.list_notes(limit=100) + with_prose = [note for note in notes if note.path != "Templates/Daily.md"] + assert with_prose + assert all(note.excerpt for note in with_prose) + + def test_backlinks_carry_it_too(self, provider) -> None: + """The second call site in the issue, and the one that cannot be paged.""" + backlinks = provider.backlinks("Areas/Work/Acme.md") + by_path = {note.path: note.excerpt for note in backlinks} + assert by_path["Areas/Work/Meetings/index.md"] == "See [[Acme Corp]]." + + def test_the_leading_heading_is_not_the_excerpt(self, provider) -> None: + """An excerpt that repeats the title is the same as not having one.""" + excerpt = self._excerpt_for(provider, "Projects/Meeting notes.md") + assert "# Meeting notes" not in excerpt + assert excerpt.startswith("Spoke to [[Acme Corp]]") + + def test_a_note_of_only_headings_gets_nothing_rather_than_its_title(self, provider) -> None: + """`Templates/Daily.md` is headings and blanks. Empty is the honest answer.""" + assert self._excerpt_for(provider, "Templates/Daily.md") == "" + + def test_a_long_opening_is_capped_and_says_so(self, provider) -> None: + provider.write("Long.md", "word " * 200, mode="create") + excerpt = self._excerpt_for(provider, "Long.md") + assert excerpt.endswith("...") + assert len(excerpt) == excerpts.OPENING_CHARS + 3 + + def test_a_search_still_quotes_the_match_not_the_opening(self, provider) -> None: + """The one call site that worked has to keep working. + + "Kickoff" sits below the four lines the opening is drawn from, so a hit + that mentions it can only have come from the match window. + """ + notes, _ = provider.search("Kickoff") + assert [note.path for note in notes] == ["Areas/Work/Acme.md"] + assert "Kickoff" in notes[0].excerpt + assert "Kickoff" not in self._excerpt_for(provider, "Areas/Work/Acme.md") + + def test_a_filter_only_search_falls_back_to_the_opening(self, provider) -> None: + """No query means no match to quote, so the opening stands in.""" + notes, _ = provider.search(tag="client") + assert [note.path for note in notes] == ["Areas/Work/Acme.md"] + assert notes[0].excerpt.startswith("The renewal is due.") + + def test_an_edit_in_obsidian_changes_the_excerpt(self, provider, obsidian_edits) -> None: + """The opening is held in the index, so it has to fall out on an edit. + + A cached excerpt that survives the note it was cut from is worse than no + excerpt: it reads as current and is not. + """ + assert self._excerpt_for(provider, "Templates/Daily.md") == "" + obsidian_edits(provider.root / "Templates" / "Daily.md", "\nCaptured the standup.\n") + provider.index.refresh(force=True) + assert self._excerpt_for(provider, "Templates/Daily.md") == "Captured the standup." + + class TestIndexFreshness: def test_a_note_written_by_obsidian_is_picked_up(self, provider) -> None: """The index cannot be built once and trusted: Obsidian edits under us.""" From 27b56e8e6ede2d57a5f8216c93aac4bbec4a0773 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 09:37:37 +0000 Subject: [PATCH 2/2] The opening a listing shows is prose, not the note's scaffolding (#6) The listing excerpt landed, and it arrived carrying the markdown around it. `opening_of` skipped the leading heading and then took four raw lines of `body`, which on the seeded vault meant `Areas/Work/Acme.md` was described as "The renewal is due. ... ## Log" and `Projects/Meeting notes.md` as its first sentence followed by "```" and the contents of the fence. Both were inherited from the `_opening` in search.py this branch moved, where they only ever reached a filter-only query. #6 puts them in front of every listed note and every backlink, which is the surface the issue argues is the whole point of the field, so they are worth correcting here rather than later. Three things, all of them the rules this package already keeps: - Code is not content. markdown.py states it and every other scanner follows it; this one read the raw body. It now picks lines on `body_scannable` and reads their text off `body`, the same pairing `title_of` and `headings_of` use, so a fence drops out and inline code keeps its text rather than the blanks that stand in for it. - A heading above the prose is still skipped; the first one after it now ends the opening instead of joining it. That is where the lead-in stops. - Blank lines no longer count against OPENING_LINES. Markdown is written double-spaced, so counting raw lines quietly meant two lines of prose on most notes. Matching `markdown._HEADING_RE` rather than `startswith("#")` also stops a note that opens on the inline tag `#meeting` losing its first line to a heading it does not have. The cost the issue asked about, measured over a 5000-note vault: per page it went down, not up. A filter-only search no longer opens fifty files to read text the index had already parsed (28.5ms -> 5.7ms); list_notes and backlinks are unchanged and now carry content. The index pays ~0.2ms/note more at build, lazily and only for notes whose mtime moved, and holds 168 bytes more per note, bounded by OPENING_CHARS and less than the headings beside it. make lint, make test (364 passed) and make smoke are green. --- knap_mcp/providers/filesystem/excerpts.py | 50 ++++++++++++++++++----- tests/test_provider.py | 46 ++++++++++++++++++++- 2 files changed, 85 insertions(+), 11 deletions(-) diff --git a/knap_mcp/providers/filesystem/excerpts.py b/knap_mcp/providers/filesystem/excerpts.py index 8ea1feb..733a7e9 100644 --- a/knap_mcp/providers/filesystem/excerpts.py +++ b/knap_mcp/providers/filesystem/excerpts.py @@ -14,6 +14,7 @@ from __future__ import annotations import re +from typing import List from . import markdown as md @@ -23,26 +24,55 @@ #: Lines of prose to draw the opening from. More than a couple, because the first #: line of a note is often a single short sentence; few enough that the cost is a -#: slice rather than a scan of a long note. +#: slice rather than a scan of a long note. Blank lines do not count against it: +#: markdown is written double-spaced, so counting raw lines would have meant two +#: lines of prose on most notes and four on the ones that happen to be dense. OPENING_LINES = 4 +# An ATX heading, mirroring the form `markdown._HEADING_RE` accepts: hashes then +# whitespace then text. Deliberately not `startswith("#")`, which would read a +# note opening on the inline tag `#meeting` as a heading and skip the line. +_HEADING_LINE_RE = re.compile(r"^#{1,6}([ \t]|$)") + def opening_of(note: md.ParsedNote, *, chars: int = OPENING_CHARS) -> str: - """The first prose of a note, flattened to one line. + """The lead-in of a note, flattened to one line. + + The frontmatter is already off ``body``. Headings above the prose go too, + because "# Meeting notes" on a note titled "Meeting notes" tells a client + nothing that `title` did not already tell it, and a summary whose excerpt + repeats its own title is the same as having no excerpt. The first heading + *after* the prose ends it instead: that is where the lead-in stops and the + note's sections begin, and "## Log" is structure, not a description. - The frontmatter is already off ``body``. Leading headings go too, because - "# Meeting notes" on a note titled "Meeting notes" tells a client nothing - that `title` did not already tell it, and a summary whose excerpt repeats its - own title is the same as having no excerpt. + Code is not content, the rule `markdown.py` states and every scanner beside + this one follows, so the code-blanked copy decides which lines are prose and + the real body supplies their text. The two share their line structure by + construction, the same trick `title_of` uses to quote a heading it matched + on the copy. Without it a note that opens on a fenced example was described + to the client as "```". Whitespace is collapsed for the same reason it is collapsed around a search match: this lands in a listing as one line, and a note that opens with an indented list or a table should not arrive full of gaps. """ - lines = note.body.strip().split("\n") - while lines and (not lines[0].strip() or lines[0].lstrip().startswith("#")): - lines.pop(0) - opening = re.sub(r"\s+", " ", " ".join(lines[:OPENING_LINES])).strip() + prose: List[str] = [] + # Not strict: the two are the same length by construction, and if that ever + # stops being true the scanners that resolve links are the place to hear + # about it. A listing should not raise over the shape of a summary field. + for line, scannable in zip( + note.body.split("\n"), note.body_scannable.split("\n"), strict=False + ): + if not scannable.strip(): + continue # blank, or a line that was nothing but code + if _HEADING_LINE_RE.match(scannable): + if prose: + break + continue + prose.append(line) + if len(prose) >= OPENING_LINES: + break + opening = re.sub(r"\s+", " ", " ".join(prose)).strip() return opening[:chars] + ("..." if len(opening) > chars else "") diff --git a/tests/test_provider.py b/tests/test_provider.py index 91c18db..e4b1846 100644 --- a/tests/test_provider.py +++ b/tests/test_provider.py @@ -305,6 +305,50 @@ def test_the_leading_heading_is_not_the_excerpt(self, provider) -> None: assert "# Meeting notes" not in excerpt assert excerpt.startswith("Spoke to [[Acme Corp]]") + def test_a_section_heading_ends_the_opening_rather_than_joining_it(self, provider) -> None: + """A heading like "## Log" describes the note's shape, not the note. + + Skipping only the *leading* heading left every interior one in the text, + so the listing #6 asked for would have answered with the lead-in plus a + stray "## Log" on any note organised into sections, which is most of them. + """ + excerpt = self._excerpt_for(provider, "Areas/Work/Acme.md") + assert excerpt == "The renewal is due. See [[Meeting notes]] and [[Something unwritten]]." + + def test_a_fenced_block_is_not_the_opening(self, provider) -> None: + """`markdown.py` says code is not content, and this is a scanner too. + + A note that opens on an example was described to the client as "```" + followed by the contents of the fence. + """ + assert "```" not in self._excerpt_for(provider, "Projects/Meeting notes.md") + provider.write( + "Fenced.md", "# Fenced\n\n```\nx = 1\n```\n\nWhat it is for.\n", mode="create" + ) + assert self._excerpt_for(provider, "Fenced.md") == "What it is for." + + def test_inline_code_keeps_its_text(self, provider) -> None: + """Blanking code decides which lines are prose; it must not eat them. + + The line is chosen on the code-blanked copy and read off the real body, + so "Run `npm install` first" arrives whole rather than as "Run first". + """ + provider.write("Inline.md", "Run `npm install` first.\n", mode="create") + assert self._excerpt_for(provider, "Inline.md") == "Run `npm install` first." + + def test_an_opening_tag_is_not_mistaken_for_a_heading(self, provider) -> None: + """`#meeting` starts with a hash and is a tag. A heading needs the space.""" + provider.write("Tagged.md", "#meeting was useful today\n", mode="create") + assert self._excerpt_for(provider, "Tagged.md") == "#meeting was useful today" + + def test_blank_lines_do_not_count_against_the_line_budget(self, provider) -> None: + """Markdown is written double-spaced, so raw lines would have halved it.""" + body = "\n\n".join(["One.", "Two.", "Three.", "Four.", "Five."]) + provider.write("Spaced.md", body, mode="create") + excerpt = self._excerpt_for(provider, "Spaced.md") + assert excerpt == "One. Two. Three. Four." + assert len(excerpt.split()) == excerpts.OPENING_LINES + def test_a_note_of_only_headings_gets_nothing_rather_than_its_title(self, provider) -> None: """`Templates/Daily.md` is headings and blanks. Empty is the honest answer.""" assert self._excerpt_for(provider, "Templates/Daily.md") == "" @@ -318,7 +362,7 @@ def test_a_long_opening_is_capped_and_says_so(self, provider) -> None: def test_a_search_still_quotes_the_match_not_the_opening(self, provider) -> None: """The one call site that worked has to keep working. - "Kickoff" sits below the four lines the opening is drawn from, so a hit + "Kickoff" sits under "## Log", past where the lead-in stops, so a hit that mentions it can only have come from the match window. """ notes, _ = provider.search("Kickoff")