Skip to content

fix(arxiv): keep the character before an inline comment - #2534

Open
alphasafal wants to merge 1 commit into
NVIDIA-NeMo:mainfrom
alphasafal:fix/arxiv-inline-comment-keeps-preceding-char
Open

alphasafal wants to merge 1 commit into
NVIDIA-NeMo:mainfrom
alphasafal:fix/arxiv-inline-comment-keeps-preceding-char

Conversation

@alphasafal

Copy link
Copy Markdown

Description

ArxivExtractor._clean_tex_file removes inline LaTeX comments with [^\\]%.+$. The [^\\] part is consumed by the match, so the character in front of every % comment is deleted along with it, and .+ needs at least one character after %, so a % at the end of a line is never removed:

Input line Before After
We use the standard setup% see the appendix We use the standard setu We use the standard setup
A line ending in a comment marker% A line ending in a comment marker% A line ending in a comment marker
Accuracy is 95\% on the test set. unchanged unchanged

The pattern is now (?<!\\)%.*$: the lookbehind keeps escaped \% without consuming the preceding character, and .* also matches a bare trailing %. Every arXiv paper that uses inline comments is affected, which is common in LaTeX sources.

This is in the same function as #2500 (the \paragraph header pattern) but on different lines, so the two do not conflict.

Usage

from nemo_curator.stages.text.download.arxiv.extract import ArxivExtractor

text = ArxivExtractor().extract({"content": ["\\section{Setup}\nWe use the standard setup% see the appendix\n"]})["text"]
assert "standard setup\n" in text  # was "standard setu\n"

Checklist

  • I am familiar with the Contributing Guide.
  • New or Existing tests cover these changes.
  • The documentation is up to date with these changes.

test_inline_comment_keeps_text_before_percent in tests/stages/text/download/arxiv/test_extract.py fails on main and passes with this change; the rest of that file passes too. ruff 0.14.10 check and format are clean. The commit is signed and signed off.

The inline-comment pattern `[^\\]%.+$` consumed the character in front of
`%`, so "setup% see appendix" became "setu", and it never matched a `%` at
the end of a line, which stayed in the text. Use a lookbehind so only the
comment is removed; escaped `\%` is still kept.

Signed-off-by: Safal Gupta <[email protected]>
@alphasafal
alphasafal requested a review from a team as a code owner October 25, 2026 14:50
@alphasafal
alphasafal requested review from a team and abhinavg4 and a balanced review from Copilot and removed request for a team October 25, 2026 14:50
@copy-pr-bot

copy-pr-bot Bot commented Oct 25, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

@greptile-apps

greptile-apps Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] This PR appears safe to merge.

Summary

Fixes arXiv comment removal so text before an inline comment stays intact.

  • Arxiv paper text keeps the character before inline comments.

No actionable issues found. Tests were inspected but not run.

Reviews (1) · Last reviewed commit: "fix(arxiv): keep the character before an..." · Reviewed by Greptile

@alphasafal

Copy link
Copy Markdown
Author

@abhinavg4 could you or another vetter approve the CI run for this? copy-pr-bot is still waiting. It's a one-line regex fix with a regression test.

@svcnvidia-nemo-ci svcnvidia-nemo-ci added waiting-on-maintainers Waiting on maintainers to respond and removed waiting-on-maintainers Waiting on maintainers to respond labels Nov 11, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community-request waiting-on-maintainers Waiting on maintainers to respond

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants