Skip to content

Commit 3dda69d

Browse files
author
Benjamin Klein
committed
fix: correct filename match detection in simple search
- Changed condition from match[0] === 0 to match[0] < positionOffset to correctly detect all matches within the filename, not just at position 0 - Added comprehensive test suite for searchSimplePost with 12 test cases covering filename matches, content matches, edge cases, and position calculations - Implemented functional prepareSimpleSearch mock for testing Fixes issue where files like '1 - Master Plan.md' were not found when searching for 'Master Plan' because the match didn't start at position 0. Addresses PR feedback: - Tests now cover off-by-one errors and boundary conditions - Match detection logic correctly identifies all filename matches - Position calculations verified through tests
1 parent 1009491 commit 3dda69d

File tree

3 files changed

+273
-3
lines changed

3 files changed

+273
-3
lines changed

mocks/obsidian.ts

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,5 +178,24 @@ export class SearchResult {
178178
export function prepareSimpleSearch(
179179
query: string
180180
): (value: string) => null | SearchResult {
181-
return null;
181+
return (text: string) => {
182+
const matches: [number, number][] = [];
183+
const lowerQuery = query.toLowerCase();
184+
const lowerText = text.toLowerCase();
185+
186+
let index = 0;
187+
while ((index = lowerText.indexOf(lowerQuery, index)) !== -1) {
188+
matches.push([index, index + query.length]);
189+
index += query.length;
190+
}
191+
192+
if (matches.length === 0) {
193+
return null;
194+
}
195+
196+
const result = new SearchResult();
197+
result.matches = matches;
198+
result.score = matches.length * -10;
199+
return result;
200+
};
182201
}

src/requestHandler.test.ts

Lines changed: 251 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -802,4 +802,255 @@ describe("requestHandler", () => {
802802
.expect(401);
803803
});
804804
});
805+
806+
describe("searchSimplePost", () => {
807+
test("match at beginning of filename", async () => {
808+
const testFile = new TFile();
809+
testFile.basename = "Master Plan";
810+
testFile.path = "Master Plan.md";
811+
812+
app.vault._markdownFiles = [testFile];
813+
app.vault._cachedRead = "Some content here";
814+
815+
const result = await request(server)
816+
.post("/search/simple/?query=Master")
817+
.set("Authorization", `Bearer ${API_KEY}`)
818+
.expect(200);
819+
820+
expect(result.body).toHaveLength(1);
821+
expect(result.body[0].filename).toBe("Master Plan.md");
822+
expect(result.body[0].matches).toHaveLength(1);
823+
expect(result.body[0].matches[0].match.source).toBe("filename");
824+
expect(result.body[0].matches[0].match.start).toBe(0);
825+
expect(result.body[0].matches[0].match.end).toBe(6);
826+
expect(result.body[0].matches[0].context).toBe("Master Plan");
827+
});
828+
829+
test("match in middle of filename", async () => {
830+
const testFile = new TFile();
831+
testFile.basename = "1 - Master Plan";
832+
testFile.path = "1 - Master Plan.md";
833+
834+
app.vault._markdownFiles = [testFile];
835+
app.vault._cachedRead = "Some content here";
836+
837+
const result = await request(server)
838+
.post("/search/simple/?query=Master")
839+
.set("Authorization", `Bearer ${API_KEY}`)
840+
.expect(200);
841+
842+
expect(result.body).toHaveLength(1);
843+
expect(result.body[0].filename).toBe("1 - Master Plan.md");
844+
expect(result.body[0].matches).toHaveLength(1);
845+
expect(result.body[0].matches[0].match.source).toBe("filename");
846+
expect(result.body[0].matches[0].match.start).toBe(4);
847+
expect(result.body[0].matches[0].match.end).toBe(10);
848+
expect(result.body[0].matches[0].context).toBe("1 - Master Plan");
849+
});
850+
851+
test("match at end of filename", async () => {
852+
const testFile = new TFile();
853+
testFile.basename = "My Master Plan";
854+
testFile.path = "My Master Plan.md";
855+
856+
app.vault._markdownFiles = [testFile];
857+
app.vault._cachedRead = "Some content here";
858+
859+
const result = await request(server)
860+
.post("/search/simple/?query=Plan")
861+
.set("Authorization", `Bearer ${API_KEY}`)
862+
.expect(200);
863+
864+
expect(result.body).toHaveLength(1);
865+
expect(result.body[0].filename).toBe("My Master Plan.md");
866+
expect(result.body[0].matches).toHaveLength(1);
867+
expect(result.body[0].matches[0].match.source).toBe("filename");
868+
expect(result.body[0].matches[0].match.start).toBe(10);
869+
expect(result.body[0].matches[0].match.end).toBe(14);
870+
expect(result.body[0].matches[0].context).toBe("My Master Plan");
871+
});
872+
873+
test("match in content only", async () => {
874+
const testFile = new TFile();
875+
testFile.basename = "Random Note";
876+
testFile.path = "Random Note.md";
877+
878+
app.vault._markdownFiles = [testFile];
879+
app.vault._cachedRead = "This is my master plan for the project.";
880+
881+
const result = await request(server)
882+
.post("/search/simple/?query=master")
883+
.set("Authorization", `Bearer ${API_KEY}`)
884+
.expect(200);
885+
886+
expect(result.body).toHaveLength(1);
887+
expect(result.body[0].filename).toBe("Random Note.md");
888+
expect(result.body[0].matches).toHaveLength(1);
889+
expect(result.body[0].matches[0].match.source).toBe("content");
890+
expect(result.body[0].matches[0].match.start).toBe(11);
891+
expect(result.body[0].matches[0].match.end).toBe(17);
892+
});
893+
894+
test("match in both filename and content", async () => {
895+
const testFile = new TFile();
896+
testFile.basename = "Master Plan";
897+
testFile.path = "Master Plan.md";
898+
899+
app.vault._markdownFiles = [testFile];
900+
app.vault._cachedRead = "The master plan is to complete this project.";
901+
902+
const result = await request(server)
903+
.post("/search/simple/?query=master")
904+
.set("Authorization", `Bearer ${API_KEY}`)
905+
.expect(200);
906+
907+
expect(result.body).toHaveLength(1);
908+
expect(result.body[0].filename).toBe("Master Plan.md");
909+
expect(result.body[0].matches).toHaveLength(2);
910+
911+
// First match should be in filename (case-insensitive)
912+
expect(result.body[0].matches[0].match.source).toBe("filename");
913+
expect(result.body[0].matches[0].match.start).toBe(0);
914+
expect(result.body[0].matches[0].match.end).toBe(6);
915+
expect(result.body[0].matches[0].context).toBe("Master Plan");
916+
917+
// Second match should be in content
918+
expect(result.body[0].matches[1].match.source).toBe("content");
919+
expect(result.body[0].matches[1].match.start).toBe(4);
920+
expect(result.body[0].matches[1].match.end).toBe(10);
921+
});
922+
923+
test("multiple matches in filename", async () => {
924+
const testFile = new TFile();
925+
testFile.basename = "Test Test Test";
926+
testFile.path = "Test Test Test.md";
927+
928+
app.vault._markdownFiles = [testFile];
929+
app.vault._cachedRead = "Content without the search term";
930+
931+
const result = await request(server)
932+
.post("/search/simple/?query=Test")
933+
.set("Authorization", `Bearer ${API_KEY}`)
934+
.expect(200);
935+
936+
expect(result.body).toHaveLength(1);
937+
expect(result.body[0].filename).toBe("Test Test Test.md");
938+
expect(result.body[0].matches).toHaveLength(3);
939+
940+
// All matches should be in filename
941+
expect(result.body[0].matches[0].match.source).toBe("filename");
942+
expect(result.body[0].matches[0].match.start).toBe(0);
943+
expect(result.body[0].matches[0].match.end).toBe(4);
944+
945+
expect(result.body[0].matches[1].match.source).toBe("filename");
946+
expect(result.body[0].matches[1].match.start).toBe(5);
947+
expect(result.body[0].matches[1].match.end).toBe(9);
948+
949+
expect(result.body[0].matches[2].match.source).toBe("filename");
950+
expect(result.body[0].matches[2].match.start).toBe(10);
951+
expect(result.body[0].matches[2].match.end).toBe(14);
952+
});
953+
954+
test("filename with special characters", async () => {
955+
const testFile = new TFile();
956+
testFile.basename = "Project (2024) - Master Plan";
957+
testFile.path = "Project (2024) - Master Plan.md";
958+
959+
app.vault._markdownFiles = [testFile];
960+
app.vault._cachedRead = "Project details";
961+
962+
const result = await request(server)
963+
.post("/search/simple/?query=2024")
964+
.set("Authorization", `Bearer ${API_KEY}`)
965+
.expect(200);
966+
967+
expect(result.body).toHaveLength(1);
968+
expect(result.body[0].filename).toBe("Project (2024) - Master Plan.md");
969+
expect(result.body[0].matches).toHaveLength(1);
970+
expect(result.body[0].matches[0].match.source).toBe("filename");
971+
expect(result.body[0].matches[0].match.start).toBe(9);
972+
expect(result.body[0].matches[0].match.end).toBe(13);
973+
expect(result.body[0].matches[0].context).toBe("Project (2024) - Master Plan");
974+
});
975+
976+
test("context length for content matches", async () => {
977+
const testFile = new TFile();
978+
testFile.basename = "Note";
979+
testFile.path = "Note.md";
980+
981+
const longContent = "A".repeat(200) + "MATCH" + "B".repeat(200);
982+
app.vault._markdownFiles = [testFile];
983+
app.vault._cachedRead = longContent;
984+
985+
const result = await request(server)
986+
.post("/search/simple/?query=MATCH&contextLength=50")
987+
.set("Authorization", `Bearer ${API_KEY}`)
988+
.expect(200);
989+
990+
expect(result.body).toHaveLength(1);
991+
expect(result.body[0].matches).toHaveLength(1);
992+
expect(result.body[0].matches[0].match.source).toBe("content");
993+
994+
// Context should be approximately 50 chars before + match + 50 chars after
995+
const context = result.body[0].matches[0].context;
996+
expect(context.length).toBeLessThanOrEqual(105); // 50 + 5 + 50
997+
expect(context).toContain("MATCH");
998+
});
999+
1000+
test("no matches returns empty array", async () => {
1001+
const testFile = new TFile();
1002+
testFile.basename = "Random Note";
1003+
testFile.path = "Random Note.md";
1004+
1005+
app.vault._markdownFiles = [testFile];
1006+
app.vault._cachedRead = "Some content";
1007+
1008+
const result = await request(server)
1009+
.post("/search/simple/?query=NonExistentTerm")
1010+
.set("Authorization", `Bearer ${API_KEY}`)
1011+
.expect(200);
1012+
1013+
expect(result.body).toHaveLength(0);
1014+
});
1015+
1016+
test("case insensitive search", async () => {
1017+
const testFile = new TFile();
1018+
testFile.basename = "MASTER Plan";
1019+
testFile.path = "MASTER Plan.md";
1020+
1021+
app.vault._markdownFiles = [testFile];
1022+
app.vault._cachedRead = "master plan details";
1023+
1024+
const result = await request(server)
1025+
.post("/search/simple/?query=master")
1026+
.set("Authorization", `Bearer ${API_KEY}`)
1027+
.expect(200);
1028+
1029+
expect(result.body).toHaveLength(1);
1030+
expect(result.body[0].matches).toHaveLength(2);
1031+
1032+
// Should match "MASTER" in filename (case-insensitive)
1033+
expect(result.body[0].matches[0].match.source).toBe("filename");
1034+
expect(result.body[0].matches[0].match.start).toBe(0);
1035+
expect(result.body[0].matches[0].match.end).toBe(6);
1036+
1037+
// Should match "master" in content
1038+
expect(result.body[0].matches[1].match.source).toBe("content");
1039+
expect(result.body[0].matches[1].match.start).toBe(0);
1040+
expect(result.body[0].matches[1].match.end).toBe(6);
1041+
});
1042+
1043+
test("unauthorized", async () => {
1044+
await request(server)
1045+
.post("/search/simple/?query=test")
1046+
.expect(401);
1047+
});
1048+
1049+
test("missing query parameter", async () => {
1050+
await request(server)
1051+
.post("/search/simple/")
1052+
.set("Authorization", `Bearer ${API_KEY}`)
1053+
.expect(400);
1054+
});
1055+
});
8051056
});

src/requestHandler.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1026,8 +1026,8 @@ export default class RequestHandler {
10261026
if (result) {
10271027
const contextMatches: SearchContext[] = [];
10281028
for (const match of result.matches) {
1029-
if (match[0] === 0) {
1030-
// When start position is between 0 and positionOffset, that means the search term matched within the headline.
1029+
if (match[0] < positionOffset) {
1030+
// When start position is between 0 and positionOffset, that means the search term matched within the filename.
10311031
contextMatches.push({
10321032
match: {
10331033
start: match[0],

0 commit comments

Comments
 (0)