Skip to content

Commit 8d72559

Browse files
timeleft--claude
andcommitted
[Bugfix #32] refactor: single-pass glob and cap candidates to 5 in _find_thought_path
- Merge two glob() calls into one traversal: collect exact match and prefix matches in a single loop; return on exact hit without scanning the rest - Cap AmbiguousThoughtID candidates at 5 entries; add total_matches field so callers can report "showing 5 of N matches" to the agent - Update test assertion to verify total_matches == 2 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent a83abb0 commit 8d72559

2 files changed

Lines changed: 37 additions & 34 deletions

File tree

src/fava_trails/trail.py

Lines changed: 36 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,14 @@
4444
class AmbiguousThoughtID(Exception):
4545
"""Raised when a shortened thought ID prefix matches more than one thought file."""
4646

47-
def __init__(self, prefix: str, candidates: list[dict]) -> None:
47+
def __init__(self, prefix: str, candidates: list[dict], total_matches: int) -> None:
4848
self.prefix = prefix
49-
self.candidates = candidates # list of {thought_id, namespace, source_type, created_at, content_preview}
49+
self.candidates = candidates # up to 5 entries: {thought_id, namespace, source_type, created_at, content_preview}
50+
self.total_matches = total_matches
51+
shown = len(candidates)
52+
detail = f"showing {shown} of {total_matches}" if total_matches > shown else f"{total_matches} matches"
5053
super().__init__(
51-
f"Prefix '{prefix}' is ambiguous — matches {len(candidates)} thoughts. "
54+
f"Prefix '{prefix}' is ambiguous — {detail}. "
5255
"Provide a longer prefix or the full ULID."
5356
)
5457

@@ -117,42 +120,41 @@ def _thought_path(self, thought_id: str, namespace: str = DEFAULT_NAMESPACE) ->
117120
def _find_thought_path(self, thought_id: str) -> Path | None:
118121
"""Find a thought file by ULID across all namespaces.
119122
120-
Supports prefix matching: if thought_id is shorter than a full ULID (26 chars),
121-
all thought files whose stem starts with the prefix are collected.
122-
- Exact match (full ULID): returns immediately.
123+
Single-pass traversal: collects exact match and prefix matches together.
124+
- Exact match (full ULID): returns immediately on first hit.
123125
- Unique prefix match: returns the single matching path.
124-
- Ambiguous prefix match: raises AmbiguousThoughtID with candidate info.
126+
- Ambiguous prefix match: raises AmbiguousThoughtID (capped at 5 candidates,
127+
total_matches count included so callers can report "showing 5 of N").
125128
- No match: returns None.
126129
"""
127-
# Fast path: exact match
130+
is_prefix = len(thought_id) < 26 # ULIDs are 26 chars
131+
prefix_matches: list[Path] = []
132+
128133
for p in self.trail_path.glob("thoughts/**/*.md"):
129134
if p.stem == thought_id:
130-
return p
131-
132-
# Prefix match (only attempted when no exact match found)
133-
if len(thought_id) < 26: # ULIDs are 26 chars
134-
matches = [
135-
p for p in self.trail_path.glob("thoughts/**/*.md")
136-
if p.stem.startswith(thought_id)
137-
]
138-
if len(matches) == 1:
139-
return matches[0]
140-
if len(matches) > 1:
141-
candidates = []
142-
for p in matches:
143-
try:
144-
record = ThoughtRecord.from_markdown(p.read_text())
145-
fm = record.frontmatter
146-
candidates.append({
147-
"thought_id": fm.thought_id,
148-
"namespace": self._get_namespace_from_path(p),
149-
"source_type": fm.source_type.value,
150-
"created_at": fm.created_at.isoformat() if fm.created_at else None,
151-
"content_preview": record.content[:100] + ("..." if len(record.content) > 100 else ""),
152-
})
153-
except Exception:
154-
candidates.append({"thought_id": p.stem, "namespace": self._get_namespace_from_path(p)})
155-
raise AmbiguousThoughtID(thought_id, candidates)
135+
return p # exact match — fast path
136+
if is_prefix and p.stem.startswith(thought_id):
137+
prefix_matches.append(p)
138+
139+
if len(prefix_matches) == 1:
140+
return prefix_matches[0]
141+
if len(prefix_matches) > 1:
142+
total = len(prefix_matches)
143+
candidates = []
144+
for p in prefix_matches[:5]:
145+
try:
146+
record = ThoughtRecord.from_markdown(p.read_text())
147+
fm = record.frontmatter
148+
candidates.append({
149+
"thought_id": fm.thought_id,
150+
"namespace": self._get_namespace_from_path(p),
151+
"source_type": fm.source_type.value,
152+
"created_at": fm.created_at.isoformat() if fm.created_at else None,
153+
"content_preview": record.content[:100] + ("..." if len(record.content) > 100 else ""),
154+
})
155+
except Exception:
156+
candidates.append({"thought_id": p.stem, "namespace": self._get_namespace_from_path(p)})
157+
raise AmbiguousThoughtID(thought_id, candidates, total_matches=total)
156158

157159
return None
158160

tests/test_tools.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1066,6 +1066,7 @@ async def test_prefix_match_ambiguous_raises(trail_manager):
10661066

10671067
err = exc_info.value
10681068
assert err.prefix == shared_prefix
1069+
assert err.total_matches == 2
10691070
assert len(err.candidates) == 2
10701071
candidate_ids = {c["thought_id"] for c in err.candidates}
10711072
assert r1.thought_id in candidate_ids

0 commit comments

Comments
 (0)