[Lanre Shittu (Shizoqua)]

Summary

Fixes #399.

DocVQA's extract_answer() scored the wrong text when a stray tag followed the final answer, so a correct answer got ANLS 0. This happened with a stray <answer> (for example <answer>42</answer> (formatted with the <answer> tag as asked)) and with a stray </answer> (for example <answer>42</answer> (wrapped in </answer> as asked)). It now takes the last complete <answer>...</answer> pair.

User impact: ANLS is the DocVQA soft score and hard is ANLS >= 0.999, so a correct answer followed by a stray tag got zero training reward and counted as a miss in reported scores.

Proof: the regression tests fail on upstream 45f65ca and pass with this change.

Root cause

extract_answer() in skillopt/envs/docvqa/evaluator.py took rfind("<answer>") and rfind("</answer>") independently and used them only when the closing tag came after the opening one. A stray <answer> after the answer made that check fail, so the last-line fallback ran. A stray </answer> after the answer passed the check but captured 42</answer> (wrapped in.

Changes

• extract_answer() matches complete pairs with <answer>((?:(?!<answer>).)*?)</answer> (case-insensitive) and takes the last match. The body cannot contain another <answer>, so nested input such as <answer>x<answer>y</answer> still returns y. The last-line fallback is unchanged when there is no complete pair.
• Stays fast on long or repetitive input: 200,000 repeated tags or a 2,000,000-character unclosed answer are extracted in at most about 0.1 s.
• Regression tests added to tests/test_docvqa_evaluator.py for both stray-tag directions, with the nested and uppercase cases as guards.
• CHANGELOG entry under Unreleased / Fixed.

Updated after review feedback (thanks Muhammad Abdullah Butt (@1AbdullahButt)) to also cover a stray </answer>.

Reproduction

from skillopt.envs.docvqa.evaluator import evaluate, extract_answer

for text in [
    "<answer>42</answer> (formatted with the <answer> tag as asked)",
    "<answer>42</answer> (wrapped in </answer> as asked)",
    "<answer>42</answer>\n<answer>",
]:
    print(repr(extract_answer(text)), evaluate(text, ["42"])["anls"])

Before:

'<answer>42</answer> (formatted with the <answer> tag as asked)' 0.0
'42</answer> (wrapped in' 0.0
'<answer>' 0.0

After: '42' 1.0 for all three.

Testing

• python -m pytest tests/test_docvqa_evaluator.py -q: on upstream 45f65ca, 5 failed and 33 passed; with the fix, 38 passed.
• python -m pytest tests/test_docvqa_evaluator.py tests/test_benchmark_task_output_paths.py -q: 54 passed.
• python -m pytest -q: 2222 passed, 12 skipped, 4 xfailed.
• python -m ruff check and python -m ruff format --check on the changed files: clean.
• python -m mkdocs build --strict: passed.

Risk and compatibility

Only responses with a stray <answer> or </answer> around the final answer change outcome, and those were scored from the wrong text before. No backend, config key, file format or CLI change.

[Muhammad Abdullah Butt (1AbdullahButt)]

The new pairing fixes the stray <answer> case. rfind("<answer>", 0, end) finds the last opening tag before the last </answer>. I traced the three new cases and the existing <answer>x</answer> then <answer>y</answer> by hand, and they give 42, 42, 42 and y.

The mirror case still fails: a stray </answer> after the answer. Take

<answer>42</answer> (wrapped in </answer> as asked)

Here end is the second </answer>, rfind("<answer>", 0, end) is 0, and the result is 42</answer> (wrapped in. That gives ANLS about 0 for a correct answer, the same failure #399 describes, only from the other side. It was also wrong on main, so this isn't a regression, but it's the same bug class and a one-line change away.

SearchQA's extract_answer (searchqa/evaluator.py:33) already avoids it by matching complete pairs: re.findall(r"<answer>(.*?)</answer>", text, re.DOTALL | re.IGNORECASE)[-1]. Used as is, that would turn your <answer>x<answer>y</answer> case into x<answer>y. Stopping the inner match at an opening tag keeps both:

_ANSWER_RE = re.compile(r"<answer>((?:(?!<answer>).)*?)</answer>", re.DOTALL | re.IGNORECASE)

def extract_answer(text: str) -> str:
    matches = _ANSWER_RE.findall(text)
    if matches:
        return matches[-1].strip()
    ...

I traced that by hand, not by running it. It gives 42 for the example above. It also gives 42 for all three of your stray-<answer> cases, y for both y cases, and Foo for the uppercase case. A test line "<answer>42</answer> (wrapped in </answer> as asked)" would pin it.

Merge note: #401, #402 and #403 all add an entry at the same spot in CHANGELOG.md, so they conflict with each other there. Each merges cleanly against main on its own. I didn't run the tests.

Reviewed with help from Claude Code.

[Lanre Shittu (Shizoqua)]

Thanks, you're right about the mirror case. Pushed d39f3c3 using your regex: a stray </answer> is now ignored too, <answer>x<answer>y</answer> still returns y, and it stays fast on long repetitive input. Added the two stray </answer> cases to the tests.