Skip to content

fix(eval): sort json2md GT when extra has no usable relation list - #4

Merged
bartrosa merged 1 commit into
mainfrom
cursor/critical-correctness-issues-f1f5
May 23, 2026
Merged

fix(eval): sort json2md GT when extra has no usable relation list#4
bartrosa merged 1 commit into
mainfrom
cursor/critical-correctness-issues-f1f5

Conversation

@cursor

@cursor cursor Bot commented May 6, 2026

Copy link
Copy Markdown

Bug and impact

bigos.eval.omnidocbench._merge_truncated_json2md silently skipped the final sort-by-order step when a manifest row had extra set to a truthy dict but extra["relation"] was missing, None, or not a list. When triggered, prepare_annos_json2md returned blocks in manifest order rather than reading order, causing the json2md GT pipeline to emit misordered GT markdown.

Concrete blast radius for OmniDocBench evaluation runs:

  • gt_markdown_json2md writes GT pieces in the wrong order for affected rows.
  • _gt_markdown_for_eval (json2md strategy, the default) feeds that misordered GT into character-level metrics — cer, ned, cer_normalized, ned_normalized, len_ratio — biasing per-sample and aggregate numbers persisted in eval/results/baseline-*.json and the markdown report.
  • _compute_per_category_metrics walks prepare_annos_json2md(row) and zips against pred blocks in reading order; misordered annos cause wrong-cell attribution of text_ned, table_teds, and formula_ned to the wrong category buckets.

Trigger: any OmniDocBench manifest row whose extra dict carries keys other than a list-typed relation. The reference tools/json2md.py always finishes with sorted(merged_annos, key=lambda x: x['order']) (documented in docs/eval/json2md-reference.md), so the previous behavior diverged from the documented contract whenever this branch was reached.

Root cause

if not extra:
    return sorted(annos, key=lambda x: int(x.get("order") or 0))
relations = extra.get("relation")
if not isinstance(relations, list):
    return annos  # <-- skips the final sort

The if not extra branch and the post-merge return both sort by order. The middle branch — taken when extra exists but doesn't contain a usable relation list — was the only exit that returned the filtered annos in their original manifest order.

Fix and validation

Sort by order in the no-relation branch as well, mirroring the reference tools/json2md.py end-of-pipeline sort.

Validation:

  • Added tests/test_eval_omnidocbench.py::test_extra_without_relation_still_sorts_by_order covering three concrete triggers: extra with only unrelated keys, extra["relation"] is None, and extra["relation"] set to a dict. Confirmed the test fails on the previous code (assert 8 < 0 from 'second\n\nfirst') and passes on the fix.
  • Full fast suite (pytest -m "not slow") green: 54 passed.
  • Full eval suite including the slow Docling smoke test green: 13 passed.

No broad refactor; the change is one branch and one regression test.

Open in Web View Automation 

_merge_truncated_json2md skipped the final sort-by-order step when row['extra']
was present but row['extra']['relation'] was missing, None, or not a list.
Real OmniDocBench manifest rows can carry an 'extra' dict that holds keys
other than 'relation' (or sets 'relation' to a non-list value), so this
branch is reachable. When triggered, prepare_annos_json2md returned blocks
in manifest order rather than reading order, which means:

- gt_markdown_json2md emitted GT markdown out of reading order.
- _gt_markdown_for_eval (json2md strategy) fed misordered GT into NED/CER
  for those rows, biasing baseline-*.json results.
- _compute_per_category_metrics zipped pred blocks against misordered GT
  annos, attributing wrong text/table/formula NED/TEDS scores to the wrong
  GT blocks.

The reference tools/json2md.py from OmniDocBench always finishes with
sorted(merged_annos, key=lambda x: x['order']) (see
docs/eval/json2md-reference.md). Mirror that ordering in the no-relation
branch by sorting before returning.

Add a regression test covering three triggers:
- extra dict with only unrelated keys
- extra['relation'] explicitly None
- extra['relation'] set to a dict instead of a list

Co-authored-by: Bartłomiej Rosa <bartrosa@users.noreply.github.com>
@bartrosa
bartrosa marked this pull request as ready for review May 23, 2026 12:45
@bartrosa
bartrosa merged commit 3ce3532 into main May 23, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants