Skip to content

fix: "\%" early stop #160

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
4ndrelim merged 3 commits into from
Apr 24, 2026
+32 −2
Merged

fix: "\%" early stop #160

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5d458c8
fix: "/%" early stop
wjiayis Apr 19, 2026
b89b0f2
Merge remote-tracking branch 'origin' into fix/gpt-early-stop
wjiayis Apr 23, 2026
918316e
feat: improve test cases
wjiayis Apr 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions internal/libs/tex/latexpand.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,23 @@ import (
"paperdebugger/internal/libs/shared"
)

// commentRegex matches a LaTeX comment: an unescaped % and everything after it
// until end of line. The leading group captures either start-of-line or a
// non-backslash character, then consumes pairs of backslashes (\\) before %.
// This generalizes to any run of N backslashes preceding %: if N is even
// (including 0), every backslash pairs up as a literal-backslash escape and
// the % is unescaped, so the comment is stripped; if N is odd, the final
// backslash escapes the % itself, so the % (and the surrounding text) is
// preserved.
var commentRegex = regexp.MustCompile(`(^|[^\\])((?:\\\\)*)%.*$`)
Copy link
Copy Markdown
Member

@4ndrelim 4ndrelim Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should cover most cases. Think its fair not to expect multiple chained \ since it hardly serves practical purpose beyond 2 consecutive backslashes which represents newline. 3 consecutive followed by a % would denote newline followed by % which is hardly the intent.

But a more general solution that guards against malicious input is to count the number of backslashes preceding % and only treating the remaining as comment if the count is even. This logic is actually implemented in the xtramcp backend in LatexParser since it is not uncommon to encounter cases e.g.\\\{. https://github.com/PaperDebugger/xtramcp/blob/main/common/latex_parser.py#L780

That said, I personally feel it is reasonable enough and am also ok with this solution.

Copy link
Copy Markdown
Member

@4ndrelim 4ndrelim Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, on a separate note, do you know why are we removing comments before passing to the LLM? I don't know the specifics, but assume this is the case. In general, even if it is a user comment not meant to be displayed, it might serve as good contextual cues or auxiliary information. The output may be informed with user's thoughts in the form of comments.

Might it be because some comments might be too long / redundant? So this is an attempt to save tokens?

Copy link
Copy Markdown
Member Author

@wjiayis wjiayis Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@4ndrelim Sure, I can generalise this solution to count odd and even backslashes.

@Junyi-99 Was there more context as to why comments were removed previously?

Copy link
Copy Markdown
Member Author

@wjiayis wjiayis Apr 23, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@4ndrelim Wait actually this already handles 3 or more backslashes before the % as well. I've improved the test cases to cover 3 or more backslashes regardless.

Copy link
Copy Markdown
Member

@4ndrelim 4ndrelim Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, i did not realise it already covers. nice.


func removeComments(text string) string {
// Split into lines, trim each line and filter empty ones
lines := strings.Split(text, "\n")
var result []string
for _, line := range lines {
trimmed := strings.TrimSpace(line)
commentRegex := regexp.MustCompile(`%.*$`)
cleaned := commentRegex.ReplaceAllString(trimmed, "")
cleaned := commentRegex.ReplaceAllString(trimmed, "$1$2")
cleaned = strings.TrimSpace(cleaned)
if len(cleaned) == 0 {
continue
Expand Down
21 changes: 21 additions & 0 deletions internal/libs/tex/latexpand_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,27 @@ Hello, world!
\end{document}`, removeComments(input))
}

func TestRemoveCommentsBackslashRunsBeforePercent(t *testing.T) {
cases := []struct {
name string
input string
want string
}{
{"1 backslash (odd) preserves %", `a\% keep`, `a\% keep`},
{"2 backslashes (even) strips comment", `a\\% drop`, `a\\`},
{"3 backslashes (odd) preserves %", `a\\\% keep`, `a\\\% keep`},
{"4 backslashes (even) strips comment", `a\\\\% drop`, `a\\\\`},
{"5 backslashes (odd) preserves %", `a\\\\\% keep`, `a\\\\\% keep`},
{"3 backslashes at line start preserves %", `\\\% keep`, `\\\% keep`},
{"4 backslashes at line start strips comment", `\\\\% drop`, `\\\\`},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
assert.Equal(t, tc.want, removeComments(tc.input))
})
}
}

func TestLatexpand(t *testing.T) {
input := map[string]string{
"main.tex": `
Expand Down
Loading