Skip to content

Commit

Permalink
feat(codeCompletion): Optimize FIM request to get appropriate suffixR…
Browse files Browse the repository at this point in the history
…eplaceRange (#3525)

* feat(codeCompletion): enhance suffix handling with unpaired brackets and semicolons

* test(codeCompletion): add tests for calculateReplaceRangeBySemiColon handling semicolons

* refactor(codeCompletion): remove unnecessary logger.info statements and add TODO for string utility

* test(codeCompletion): expand tests for calculateReplaceRangeBySemiColon to cover various semicolon scenarios

* refactor(codeCompletion): simplify suffix handling and improve line end detection

* refactor(string): remove redundant handling of semicolons in findUnpairedAutoClosingChars
  • Loading branch information
Sma1lboy authored Dec 30, 2024
1 parent 3760583 commit eae43ba
Show file tree
Hide file tree
Showing 7 changed files with 216 additions and 18 deletions.
15 changes: 11 additions & 4 deletions clients/tabby-agent/src/codeCompletion/contexts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@ export class CompletionContext {
currSeg: string = "";
withCorrectCompletionItem: boolean = false; // weather we are using completionItem or not

// is current suffix is at end of line excluding auto closed char
lineEnd: RegExpMatchArray | null = null;

constructor(request: CompletionRequest) {
this.filepath = request.filepath;
this.language = request.language;
Expand All @@ -119,13 +122,13 @@ export class CompletionContext {
this.relevantSnippetsFromChangedFiles = request.relevantSnippetsFromChangedFiles;
this.snippetsFromOpenedFiles = request.relevantSnippetsFromOpenedFiles;

const lineEnd = isAtLineEndExcludingAutoClosedChar(this.currentLineSuffix);
this.mode = lineEnd ? "default" : "fill-in-line";
this.lineEnd = isAtLineEndExcludingAutoClosedChar(this.currentLineSuffix);
this.mode = this.lineEnd ? "default" : "fill-in-line";
this.hash = hashObject({
filepath: this.filepath,
language: this.language,
prefix: this.prefix,
currentLineSuffix: lineEnd ? "" : this.currentLineSuffix,
currentLineSuffix: this.lineEnd ? "" : this.currentLineSuffix,
nextLines: this.suffixLines.slice(1).join(""),
position: this.position,
clipboard: this.clipboard,
Expand Down Expand Up @@ -209,7 +212,11 @@ export class CompletionContext {
buildSegments(config: ConfigData["completion"]["prompt"]): TabbyApiComponents["schemas"]["Segments"] {
// prefix && suffix
const prefix = this.prefixLines.slice(Math.max(this.prefixLines.length - config.maxPrefixLines, 0)).join("");
const suffix = this.suffixLines.slice(0, config.maxSuffixLines).join("");
let suffix = this.suffixLines.slice(0, config.maxSuffixLines).join("");
// if it's end of line, we don't need to include the suffix
if (this.lineEnd) {
suffix = "\n" + suffix.split("\n").slice(1).join("\n");
}

// filepath && git_url
let relativeFilepathRoot: string | undefined = undefined;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { PostprocessFilter } from "./base";
import { CompletionItem } from "../solution";
import { calculateReplaceRangeByBracketStack } from "./calculateReplaceRangeByBracketStack";
import { calculateReplaceRangeBySemiColon } from "./calculateReplaceRangeBySemiColon";

export function calculateReplaceRange(): PostprocessFilter {
return async (item: CompletionItem): Promise<CompletionItem> => {
return calculateReplaceRangeByBracketStack(item);
const afterBracket = calculateReplaceRangeByBracketStack(item);
return calculateReplaceRangeBySemiColon(afterBracket);
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,43 @@ describe("postprocess", () => {
};
await assertFilterResult(filter, context, completion, expected);
});
it("should handle bracket case with semicolon", async () => {
const context = documentContext`
console.log("║");
`;
context.language = "typescript";
const completion = {
text: inline`
├hello world");┤
`,
};
const expected = {
text: inline`
├hello world");┤
`,
replaceSuffix: '");',
};
await assertFilterResult(filter, context, completion, expected);
});
it("should handle bracket case with semicolon", async () => {
const context = documentContext`
console.log(║);
`;
context.language = "typescript";
const completion = {
text: inline`
├a + b);┤
`,
};
const expected = {
text: inline`
├a + b);┤
`,
replaceSuffix: ");",
};
await assertFilterResult(filter, context, completion, expected);
});
});

describe("calculateReplaceRangeByBracketStack: bad cases", () => {
const filter = calculateReplaceRangeByBracketStack;
it("cannot handle the case of completion bracket stack is same with suffix but should not be replaced", async () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,31 +1,28 @@
import { logger } from "./base";
import { CompletionItem } from "../solution";
import { isBlank, findUnpairedAutoClosingChars } from "../../utils/string";

export function calculateReplaceRangeByBracketStack(item: CompletionItem): CompletionItem {
const context = item.context;
const { currentLineSuffix } = context;
const suffixText = currentLineSuffix.trimEnd();
if (isBlank(suffixText)) {
return item;
}
const unpaired = findUnpairedAutoClosingChars(item.text).join("");
if (isBlank(unpaired)) {

if (!context.lineEnd) {
return item;
}

let modified: CompletionItem | undefined = undefined;
if (suffixText.startsWith(unpaired)) {
modified = item.withSuffix(unpaired);
} else if (unpaired.startsWith(suffixText)) {
const suffixText = context.currentLineSuffix.trimEnd();
const lineEnd = context.lineEnd[0];
if (suffixText.startsWith(lineEnd)) {
modified = item.withSuffix(lineEnd);
} else if (lineEnd.startsWith(suffixText)) {
modified = item.withSuffix(suffixText);
}
if (modified) {
logger.trace("Adjust replace range by bracket stack.", {
position: context.position,
currentLineSuffix,
completionText: item.text,
unpaired,
lineEnd,
replaceSuffix: item.replaceSuffix,
});
return modified;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
import { documentContext, inline, assertFilterResult } from "./testUtils";
import { calculateReplaceRangeBySemiColon } from "./calculateReplaceRangeBySemiColon";

describe("postprocess", () => {
describe("calculateReplaceRangeBySemiColon", () => {
const filter = calculateReplaceRangeBySemiColon;

it("should handle semicolon in string concatenation", async () => {
const context = documentContext`
const content = "hello world";
const a = "nihao" + ║;
`;
context.language = "typescript";
const completion = {
text: inline`
├content;┤
`,
};
const expected = {
text: inline`
├content;┤
`,
replaceSuffix: ";",
};
await assertFilterResult(filter, context, completion, expected);
});

it("should handle semicolon at the end of a statement", async () => {
const context = documentContext`
const content = "hello world"║;
`;
context.language = "typescript";
const completion = {
text: inline`
├;┤
`,
};
const expected = {
text: inline`
├;┤
`,
replaceSuffix: ";",
};
await assertFilterResult(filter, context, completion, expected);
});

it("should not handle any semicolon at the end of a statement", async () => {
const context = documentContext`
const content = "hello world"║
`;
context.language = "typescript";
const completion = {
text: inline`
├┤
`,
};
const expected = {
text: inline`
├┤
`,
replaceSuffix: "",
};
await assertFilterResult(filter, context, completion, expected);
});

it("should not modify if no semicolon in completion text", async () => {
const context = documentContext`
const content = "hello world"║
`;
context.language = "typescript";
const completion = {
text: inline`
├content┤
`,
};
const expected = {
text: inline`
├content┤
`,
replaceSuffix: "",
};
await assertFilterResult(filter, context, completion, expected);
});

it("should handle multiple semicolons in completion text", async () => {
const context = documentContext`
const content = "hello world"║;
`;
context.language = "typescript";
const completion = {
text: inline`
├content;;┤
`,
};
const expected = {
text: inline`
├content;;┤
`,
replaceSuffix: ";",
};
await assertFilterResult(filter, context, completion, expected);
});

it("should handle semicolon in the middle of a statement", async () => {
const context = documentContext`
const content = "hello; world"║;
`;
context.language = "typescript";
const completion = {
text: inline`
├content;┤
`,
};
const expected = {
text: inline`
├content;┤
`,
replaceSuffix: ";",
};
await assertFilterResult(filter, context, completion, expected);
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { isBlank } from "../../utils/string";
import { CompletionItem } from "../solution";
import { logger } from "./base";

export function calculateReplaceRangeBySemiColon(item: CompletionItem): CompletionItem {
const context = item.context;
const { currentLineSuffix } = context;
const suffixText = currentLineSuffix.trimEnd();

if (isBlank(suffixText)) {
return item;
}

const completionText = item.text.trimEnd();
if (!completionText.includes(";")) {
return item;
}

if (suffixText.startsWith(";") && completionText.endsWith(";")) {
const modified = item.withSuffix(";");

logger.trace("Adjust replace range by semicolon.", {
position: context.position,
currentLineSuffix,
completionText: item.text,
modifiedText: item.text,
replaceSuffix: modified.replaceSuffix,
});

return modified;
}

return item;
}
2 changes: 1 addition & 1 deletion clients/tabby-agent/src/utils/string.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ export const autoClosingPairs: AutoClosingPair[] = [
},
];

export const regOnlyAutoClosingCloseChars = /^([)\]}>"'`]|(\/>))*$/g;
export const regOnlyAutoClosingCloseChars = /^([)\]}>"'`]|(\/>)|[;,])*$/g;

// FIXME: This function is not good enough, it can not handle escaped characters.
export function findUnpairedAutoClosingChars(input: string): string[] {
Expand Down

0 comments on commit eae43ba

Please sign in to comment.