Include the title of the file in the simple search #187

phortx · 2025-09-17T18:27:30Z

... also some smaller formatting fixes that the IDE did.

See #172

src/requestHandler.ts

phortx · 2025-10-30T05:20:57Z

Hi @coddingtonbear,

what do you think? Could we merge this by time? Do you need anything from my side? :)

src/requestHandler.ts

Copilot

Pull Request Overview

This PR enhances the search functionality to include file names in the search scope and distinguish between filename and content matches. The implementation prepends file basenames to the search text and handles match position offsets accordingly.

Added a source field to SearchContext to distinguish between filename and content matches
Modified search logic to prepend file basename to searchable text and adjust match positions
Fixed several formatting issues (whitespace, spacing) in the mock files

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
src/types.ts	Added `source` field to `SearchContext` interface to distinguish match origin
src/requestHandler.ts	Enhanced search to include filenames and differentiate match sources; fixed contextLength parsing bug
mocks/obsidian.ts	Fixed spacing/formatting inconsistencies; added `basename` property to TFile mock
docs/openapi.yaml	Updated API documentation with descriptions and new `source` field

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/requestHandler.ts

Copilot

Pull Request Overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/requestHandler.ts

mocks/obsidian.ts

coddingtonbear

Just a couple more comments -- once you've updated the mocks and (I assume) your tests, let me know!

coddingtonbear · 2025-11-15T19:04:48Z

mocks/obsidian.ts

  query: string
 ): (value: string) => null | SearchResult {
-  return null;
+  return (text: string) => {


Just a quick note about this file, @phortx: since it lives under mocks/, it’s meant purely as a mock—basically something lightweight so the type checker and tests have a stand-in. We shouldn’t put any real implementation logic here.

Instead, the idea is that your tests can control what this mock returns in different scenarios. If you take a peek at some of the other mock functions in mocks/obsidian.ts, you’ll see how they’re set up to let tests override behavior as needed.

So to make this consistent: this function shouldn’t implement any real behavior. It should either do nothing or return whatever value your test explicitly configures, so that you can verify how requestHandler.ts responds to those different return values.

Good point, I resetted the mock implementation and added a way for the tests to override the behavior.

Co-authored-by: Copilot <[email protected]>

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 9 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

Copilot · 2025-12-01T20:04:48Z

src/requestHandler.test.ts

+  describe("searchSimplePost", () => {
+    beforeEach(() => {
+      // Setup mock for prepareSimpleSearch
+      _prepareSimpleSearchMock.behavior = (query: string) => {
+        const queryLower = query.toLowerCase();
+        const queryLength = query.length;
+        return (text: string) => {
+          const textLower = text.toLowerCase();
+          const matches: [number, number][] = [];
+          let index = 0;
+
+          // Find all matches (case-insensitive)
+          while ((index = textLower.indexOf(queryLower, index)) !== -1) {
+            matches.push([index, index + queryLength]);
+            index += 1;
+          }
+
+          if (matches.length === 0) {
+            return null;
+          }
+
+          // Calculate score based on number of matches
+          const score = matches.length;
+
+          return {
+            score,
+            matches,
+          } as SearchResult;
+        };
+      };
+    });
+
+    afterEach(() => {
+      // Clean up mock
+      _prepareSimpleSearchMock.behavior = null;
+    });
+
+    test("match at beginning of filename", async () => {
+      const testFile = new TFile();
+      testFile.basename = "Master Plan";
+      testFile.path = "Master Plan.md";
+
+      app.vault._markdownFiles = [testFile];
+      app.vault._cachedRead = "Some content here";
+
+      const result = await request(server)
+        .post("/search/simple/?query=Master")
+        .set("Authorization", `Bearer ${API_KEY}`)
+        .expect(200);
+
+      expect(result.body).toHaveLength(1);
+      expect(result.body[0].filename).toBe("Master Plan.md");
+      expect(result.body[0].matches).toHaveLength(1);
+      expect(result.body[0].matches[0].match.source).toBe("filename");
+      expect(result.body[0].matches[0].match.start).toBe(0);
+      expect(result.body[0].matches[0].match.end).toBe(6);
+      expect(result.body[0].matches[0].context).toBe("Master Plan");
+    });
+
+    test("match in middle of filename", async () => {
+      const testFile = new TFile();
+      testFile.basename = "1 - Master Plan";
+      testFile.path = "1 - Master Plan.md";
+
+      app.vault._markdownFiles = [testFile];
+      app.vault._cachedRead = "Some content here";
+
+      const result = await request(server)
+        .post("/search/simple/?query=Master")
+        .set("Authorization", `Bearer ${API_KEY}`)
+        .expect(200);
+
+      expect(result.body).toHaveLength(1);
+      expect(result.body[0].filename).toBe("1 - Master Plan.md");
+      expect(result.body[0].matches).toHaveLength(1);
+      expect(result.body[0].matches[0].match.source).toBe("filename");
+      expect(result.body[0].matches[0].match.start).toBe(4);
+      expect(result.body[0].matches[0].match.end).toBe(10);
+      expect(result.body[0].matches[0].context).toBe("1 - Master Plan");
+    });
+
+    test("match at end of filename", async () => {
+      const testFile = new TFile();
+      testFile.basename = "My Master Plan";
+      testFile.path = "My Master Plan.md";
+
+      app.vault._markdownFiles = [testFile];
+      app.vault._cachedRead = "Some content here";
+
+      const result = await request(server)
+        .post("/search/simple/?query=Plan")
+        .set("Authorization", `Bearer ${API_KEY}`)
+        .expect(200);
+
+      expect(result.body).toHaveLength(1);
+      expect(result.body[0].filename).toBe("My Master Plan.md");
+      expect(result.body[0].matches).toHaveLength(1);
+      expect(result.body[0].matches[0].match.source).toBe("filename");
+      expect(result.body[0].matches[0].match.start).toBe(10);
+      expect(result.body[0].matches[0].match.end).toBe(14);
+      expect(result.body[0].matches[0].context).toBe("My Master Plan");
+    });
+
+    test("match in content only", async () => {
+      const testFile = new TFile();
+      testFile.basename = "Random Note";
+      testFile.path = "Random Note.md";
+
+      app.vault._markdownFiles = [testFile];
+      app.vault._cachedRead = "This is my master plan for the project.";
+
+      const result = await request(server)
+        .post("/search/simple/?query=master")
+        .set("Authorization", `Bearer ${API_KEY}`)
+        .expect(200);
+
+      expect(result.body).toHaveLength(1);
+      expect(result.body[0].filename).toBe("Random Note.md");
+      expect(result.body[0].matches).toHaveLength(1);
+      expect(result.body[0].matches[0].match.source).toBe("content");
+      expect(result.body[0].matches[0].match.start).toBe(11);
+      expect(result.body[0].matches[0].match.end).toBe(17);
+    });
+
+    test("match in both filename and content", async () => {
+      const testFile = new TFile();
+      testFile.basename = "Master Plan";
+      testFile.path = "Master Plan.md";
+
+      app.vault._markdownFiles = [testFile];
+      app.vault._cachedRead = "The master plan is to complete this project.";
+
+      const result = await request(server)
+        .post("/search/simple/?query=master")
+        .set("Authorization", `Bearer ${API_KEY}`)
+        .expect(200);
+
+      expect(result.body).toHaveLength(1);
+      expect(result.body[0].filename).toBe("Master Plan.md");
+      expect(result.body[0].matches).toHaveLength(2);
+
+      // First match should be in filename (case-insensitive)
+      expect(result.body[0].matches[0].match.source).toBe("filename");
+      expect(result.body[0].matches[0].match.start).toBe(0);
+      expect(result.body[0].matches[0].match.end).toBe(6);
+      expect(result.body[0].matches[0].context).toBe("Master Plan");
+
+      // Second match should be in content
+      expect(result.body[0].matches[1].match.source).toBe("content");
+      expect(result.body[0].matches[1].match.start).toBe(4);
+      expect(result.body[0].matches[1].match.end).toBe(10);
+    });
+
+    test("multiple matches in filename", async () => {
+      const testFile = new TFile();
+      testFile.basename = "Test Test Test";
+      testFile.path = "Test Test Test.md";
+
+      app.vault._markdownFiles = [testFile];
+      app.vault._cachedRead = "Content without the search term";
+
+      const result = await request(server)
+        .post("/search/simple/?query=Test")
+        .set("Authorization", `Bearer ${API_KEY}`)
+        .expect(200);
+
+      expect(result.body).toHaveLength(1);
+      expect(result.body[0].filename).toBe("Test Test Test.md");
+      expect(result.body[0].matches).toHaveLength(3);
+
+      // All matches should be in filename
+      expect(result.body[0].matches[0].match.source).toBe("filename");
+      expect(result.body[0].matches[0].match.start).toBe(0);
+      expect(result.body[0].matches[0].match.end).toBe(4);
+
+      expect(result.body[0].matches[1].match.source).toBe("filename");
+      expect(result.body[0].matches[1].match.start).toBe(5);
+      expect(result.body[0].matches[1].match.end).toBe(9);
+
+      expect(result.body[0].matches[2].match.source).toBe("filename");
+      expect(result.body[0].matches[2].match.start).toBe(10);
+      expect(result.body[0].matches[2].match.end).toBe(14);
+    });
+
+    test("filename with special characters", async () => {
+      const testFile = new TFile();
+      testFile.basename = "Project (2024) - Master Plan";
+      testFile.path = "Project (2024) - Master Plan.md";
+
+      app.vault._markdownFiles = [testFile];
+      app.vault._cachedRead = "Project details";
+
+      const result = await request(server)
+        .post("/search/simple/?query=2024")
+        .set("Authorization", `Bearer ${API_KEY}`)
+        .expect(200);
+
+      expect(result.body).toHaveLength(1);
+      expect(result.body[0].filename).toBe("Project (2024) - Master Plan.md");
+      expect(result.body[0].matches).toHaveLength(1);
+      expect(result.body[0].matches[0].match.source).toBe("filename");
+      expect(result.body[0].matches[0].match.start).toBe(9);
+      expect(result.body[0].matches[0].match.end).toBe(13);
+      expect(result.body[0].matches[0].context).toBe("Project (2024) - Master Plan");
+    });
+
+    test("context length for content matches", async () => {
+      const testFile = new TFile();
+      testFile.basename = "Note";
+      testFile.path = "Note.md";
+
+      const longContent = "A".repeat(200) + "MATCH" + "B".repeat(200);
+      app.vault._markdownFiles = [testFile];
+      app.vault._cachedRead = longContent;
+
+      const result = await request(server)
+        .post("/search/simple/?query=MATCH&contextLength=50")
+        .set("Authorization", `Bearer ${API_KEY}`)
+        .expect(200);
+
+      expect(result.body).toHaveLength(1);
+      expect(result.body[0].matches).toHaveLength(1);
+      expect(result.body[0].matches[0].match.source).toBe("content");
+
+      // Context should be approximately 50 chars before + match + 50 chars after
+      const context = result.body[0].matches[0].context;
+      expect(context.length).toBeLessThanOrEqual(105); // 50 + 5 + 50
+      expect(context).toContain("MATCH");
+    });
+
+    test("no matches returns empty array", async () => {
+      const testFile = new TFile();
+      testFile.basename = "Random Note";
+      testFile.path = "Random Note.md";
+
+      app.vault._markdownFiles = [testFile];
+      app.vault._cachedRead = "Some content";
+
+      const result = await request(server)
+        .post("/search/simple/?query=NonExistentTerm")
+        .set("Authorization", `Bearer ${API_KEY}`)
+        .expect(200);
+
+      expect(result.body).toHaveLength(0);
+    });
+
+    test("case insensitive search", async () => {
+      const testFile = new TFile();
+      testFile.basename = "MASTER Plan";
+      testFile.path = "MASTER Plan.md";
+
+      app.vault._markdownFiles = [testFile];
+      app.vault._cachedRead = "master plan details";
+
+      const result = await request(server)
+        .post("/search/simple/?query=master")
+        .set("Authorization", `Bearer ${API_KEY}`)
+        .expect(200);
+
+      expect(result.body).toHaveLength(1);
+      expect(result.body[0].matches).toHaveLength(2);
+
+      // Should match "MASTER" in filename (case-insensitive)
+      expect(result.body[0].matches[0].match.source).toBe("filename");
+      expect(result.body[0].matches[0].match.start).toBe(0);
+      expect(result.body[0].matches[0].match.end).toBe(6);
+
+      // Should match "master" in content
+      expect(result.body[0].matches[1].match.source).toBe("content");
+      expect(result.body[0].matches[1].match.start).toBe(0);
+      expect(result.body[0].matches[1].match.end).toBe(6);
+    });
+
+    test("unauthorized", async () => {
+      await request(server)
+        .post("/search/simple/?query=test")
+        .expect(401);
+    });
+
+    test("missing query parameter", async () => {
+      await request(server)
+        .post("/search/simple/")
+        .set("Authorization", `Bearer ${API_KEY}`)
+        .expect(400);
+    });
+  });


Missing test coverage for the edge case where a search term spans the boundary between the filename and content. For example, if the filename is "Master" and the content starts with "Plan", searching for "ter\n\nPl" would create a match that spans both. This edge case should be tested to ensure the implementation handles it correctly (or explicitly rejects it).

Copilot · 2025-12-01T20:04:48Z

src/requestHandler.ts

-    const contextLength: number =
-      parseInt(req.query.contextLength as string, 10) ?? 100;
+    const contextLengthRaw = parseInt(req.query.contextLength as string, 10);
+    const contextLength = Number.isNaN(contextLengthRaw) ? 100 : contextLengthRaw;


The contextLength parameter is not validated for negative or unreasonably large values. A negative value would cause Math.max(match[0] - positionOffset - contextLength, 0) to potentially return a larger start position than expected, and the slice end position match[1] - positionOffset + contextLength could be negative. Consider adding validation to ensure contextLength >= 0 or clamping it to a reasonable range.

Suggested change

const contextLength = Number.isNaN(contextLengthRaw) ? 100 : contextLengthRaw;

// Clamp contextLength to [0, 1000], default to 100 if invalid

const contextLength =

Number.isNaN(contextLengthRaw) || contextLengthRaw < 0

? 100

: Math.min(contextLengthRaw, 1000);

Copilot · 2025-12-01T20:04:48Z

docs/openapi.yaml

+                                description: "End character position of the match within the file. Starts with 0."
                              start:
                                type: "number"
+                                description: "Start character position of the match within the file. Starts with 0."


The description "Start character position of the match within the file. Starts with 0." is now ambiguous. When source is "filename", the position is within the filename (basename), not the file content. When source is "content", it's within the file content. Consider updating the description to clarify this: "Start character position of the match. When source is 'filename', this is the position within the filename. When source is 'content', this is the position within the file content. Starts with 0."

Suggested change

description: "Start character position of the match within the file. Starts with 0."

description: "Start character position of the match. When source is 'filename', this is the position within the filename. When source is 'content', this is the position within the file content. Starts with 0."

Copilot · 2025-12-01T20:04:49Z

src/requestHandler.ts

-          });
+          // Check if the entire match is within the filename (including the newlines).
+          // We need to ensure both start and end positions are within the filename prefix.
+          if (match[0] < positionOffset && match[1] <= positionOffset) {


The condition doesn't handle matches that span across the boundary between filename and content. If a search term starts in the filename but ends in the content (e.g., searching for "Master\n\nThe" when filename is "Master"), the match would have match[0] < positionOffset but match[1] > positionOffset. This case falls into the else branch and would produce incorrect results because:

It would be marked as source: "content" when it actually spans both

The position offset subtraction would make start negative

Consider adding a condition to handle boundary-spanning matches, or explicitly filter them out if they shouldn't be supported.

Copilot · 2025-12-01T20:04:49Z

src/types.ts

  match: {
    start: number;
    end: number;
+    source: string;


The source field is typed as string, but based on the implementation it should only ever be "filename" or "content". Consider using a union type "filename" | "content" for better type safety and to make the API contract clearer.

Suggested change

source: string;

source: "filename" | "content";

Copilot · 2025-12-01T20:04:49Z

docs/openapi.yaml

                                type: "number"
+                                description: "Start character position of the match within the file. Starts with 0."
+                              source:
+                                type: "string"


The source field description says "'filename' or 'content'" but the type is just string. Consider using an enum to be more precise in the OpenAPI spec. Example:

source: type: "string" enum: ["filename", "content"] description: "Where the search term matched"

Suggested change

type: "string"

type: "string"

enum: ["filename", "content"]

Copilot · 2025-12-01T20:04:50Z

docs/openapi.yaml

                            properties:
                              end:
                                type: "number"
+                                description: "End character position of the match within the file. Starts with 0."


The description "End character position of the match within the file. Starts with 0." is now ambiguous. When source is "filename", the position is within the filename (basename), not the file content. When source is "content", it's within the file content. Consider updating the description to clarify this: "End character position of the match. When source is 'filename', this is the position within the filename. When source is 'content', this is the position within the file content. Starts with 0."

Suggested change

description: "End character position of the match within the file. Starts with 0."

description: "End character position of the match. When source is 'filename', this is the position within the filename. When source is 'content', this is the position within the file content. Starts with 0."

Copilot · 2025-12-01T20:04:50Z

mocks/obsidian.ts

+  if (_prepareSimpleSearchMock.behavior) {
+    return _prepareSimpleSearchMock.behavior(query);
+  }
  return null;


The return type is (value: string) => null | SearchResult, which means the function should return another function. However, return null; returns null directly, not a function. This should be return () => null; to match the return type and maintain consistency with the mock behavior pattern.

Suggested change

return null;

return () => null;

Copilot · 2025-12-01T20:04:50Z

src/requestHandler.ts

+            contextMatches.push({
+              match: {
+                start: match[0],
+                end: Math.min(match[1], file.basename.length),
+                source: "filename"
+              },
+              context: file.basename,
+            });


If file.basename is empty, filenamePrefix would be "\n\n". A match could occur in these newline characters and be classified as a "filename" match with context: "". While this is technically correct (searching for newlines), it might lead to confusing results. Consider filtering out matches that occur entirely within the newlines but not in the actual basename text (i.e., when match[0] >= file.basename.length).

Suggested change

contextMatches.push({

match: {

start: match[0],

end: Math.min(match[1], file.basename.length),

source: "filename"

},

context: file.basename,

});

// If the basename is empty and the match occurs only in the newlines, skip this match.

if (!(file.basename.length === 0 && match[0] >= file.basename.length)) {

contextMatches.push({

match: {

start: match[0],

end: Math.min(match[1], file.basename.length),

source: "filename"

},

context: file.basename,

});

}

coddingtonbear reviewed Sep 17, 2025

View reviewed changes

src/requestHandler.ts Outdated Show resolved Hide resolved

coddingtonbear added the needs more discussion More information or discussion is necessary before proceeding label Sep 28, 2025

phortx force-pushed the main branch from f4e1619 to 572d091 Compare October 7, 2025 06:43

coddingtonbear reviewed Nov 1, 2025

View reviewed changes

src/requestHandler.ts Outdated Show resolved Hide resolved

coddingtonbear requested a review from Copilot November 4, 2025 17:11

Copilot AI reviewed Nov 4, 2025

View reviewed changes

src/requestHandler.ts Outdated Show resolved Hide resolved

src/requestHandler.ts Outdated Show resolved Hide resolved

src/requestHandler.ts Outdated Show resolved Hide resolved

src/requestHandler.ts Outdated Show resolved Hide resolved

coddingtonbear requested a review from Copilot November 15, 2025 19:02

Copilot started reviewing on behalf of coddingtonbear November 15, 2025 19:03 View session

Copilot finished reviewing on behalf of coddingtonbear November 15, 2025 19:06

Copilot AI reviewed Nov 15, 2025

View reviewed changes

src/requestHandler.ts Outdated Show resolved Hide resolved

src/requestHandler.ts Outdated Show resolved Hide resolved

src/requestHandler.ts Outdated Show resolved Hide resolved

src/requestHandler.ts Outdated Show resolved Hide resolved

mocks/obsidian.ts Outdated Show resolved Hide resolved

coddingtonbear requested changes Nov 15, 2025

View reviewed changes

feat: include the title of the file in the simple search

4ba0b99

phortx force-pushed the main branch from 05d7304 to 4ba0b99 Compare November 21, 2025 15:37

phortx and others added 2 commits November 21, 2025 16:38

fix: slice calculation in src/requestHandler.ts

1e6aae1

Co-authored-by: Copilot <[email protected]>

fix: tests

7e6d10f

phortx requested a review from coddingtonbear November 21, 2025 15:47

coddingtonbear requested a review from Copilot December 1, 2025 19:59

Copilot started reviewing on behalf of coddingtonbear December 1, 2025 19:59 View session

Copilot finished reviewing on behalf of coddingtonbear December 1, 2025 20:03

Copilot AI reviewed Dec 1, 2025

View reviewed changes

-    const contextLength = Number.isNaN(contextLengthRaw) ? 100 : contextLengthRaw;
+    // Clamp contextLength to [0, 1000], default to 100 if invalid
+    const contextLength =
+      Number.isNaN(contextLengthRaw) || contextLengthRaw < 0
+        ? 100
+        : Math.min(contextLengthRaw, 1000);

	description: "Start character position of the match within the file. Starts with 0."
	description: "Start character position of the match. When source is 'filename', this is the position within the filename. When source is 'content', this is the position within the file content. Starts with 0."

	description: "End character position of the match within the file. Starts with 0."
	description: "End character position of the match. When source is 'filename', this is the position within the filename. When source is 'content', this is the position within the file content. Starts with 0."

Include the title of the file in the simple search #187

Are you sure you want to change the base?

Include the title of the file in the simple search #187

Uh oh!

Conversation

phortx commented Sep 17, 2025

Uh oh!

Uh oh!

phortx commented Oct 30, 2025

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coddingtonbear left a comment

Choose a reason for hiding this comment

Uh oh!

coddingtonbear Nov 15, 2025

Choose a reason for hiding this comment

Uh oh!

phortx Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants