kenzychew commited on
Commit
fa6cf50
·
1 Parent(s): bd211ba

eval: add --retry-errors to re-predict only the entries that failed

Browse files

A quota outage left 44 of 361 cached SROIE documents with no extraction.
Recovering them with --overwrite would have re-run all 361 and replaced the
317 successful predictions with fresh model output, invalidating every
before/after rule comparison built on them -- S2 and the monetary tolerance
change both depend on the predictions being frozen while only the rules vary.

--retry-errors selects exactly the entries whose error field is set and skips
every other cached id, so a successful prediction is never re-run, re-billed,
or rewritten. Two guards, because both failure modes here spend quota
silently: combining it with --overwrite raises rather than letting one win,
and using it against an empty cache raises rather than falling through to a
full predict.

Note this leaves a cache that is mixed by construction: the backfilled 44
carry confidence and validation computed under current rules, the original
317 under the rules in force when they ran. That is only a disk-level
inconsistency. Scoring with --revalidate recomputes both from the cached
predicted document, which is model output and therefore rule-independent, so
every entry is judged under one rule set regardless of when it was predicted.
The drift check reports the disk-level disagreement rather than hiding it.

Files changed (4) hide show
  1. eval/cache.py +19 -0
  2. eval/predict.py +33 -3
  3. eval/run_eval.py +11 -0
  4. tests/test_eval.py +48 -1
eval/cache.py CHANGED
@@ -90,6 +90,25 @@ def existing_ids(cache_base: Path, dataset: str) -> set[str]:
90
  return {str(entry["id"]) for entry in read_entries(cache_base, dataset)}
91
 
92
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
93
  def report_from_dict(validation: dict[str, Any]) -> ValidationReport:
94
  """Reconstruct a :class:`ValidationReport` from its cached dict form.
95
 
 
90
  return {str(entry["id"]) for entry in read_entries(cache_base, dataset)}
91
 
92
 
93
+ def errored_ids(cache_base: Path, dataset: str) -> set[str]:
94
+ """Return the ids of cached entries whose pipeline run recorded an error.
95
+
96
+ These are documents that produced no extraction -- a quota outage, a
97
+ timeout, an unreadable file -- as distinct from documents the model read
98
+ and the rules then rejected. Only these are worth re-running: a successful
99
+ prediction must stay frozen so that before/after rule comparisons remain
100
+ attributable to the rule change.
101
+
102
+ Args:
103
+ cache_base: Root cache directory.
104
+ dataset: Dataset name (subdirectory).
105
+
106
+ Returns:
107
+ The set of example ids with a non-empty ``error`` field.
108
+ """
109
+ return {str(e["id"]) for e in read_entries(cache_base, dataset) if e.get("error")}
110
+
111
+
112
  def report_from_dict(validation: dict[str, Any]) -> ValidationReport:
113
  """Reconstruct a :class:`ValidationReport` from its cached dict form.
114
 
eval/predict.py CHANGED
@@ -25,7 +25,7 @@ from doc_agent.backends.base import create_backend
25
  from doc_agent.config import Settings, load_config
26
  from doc_agent.core import process_document
27
 
28
- from eval.cache import DEFAULT_CACHE_BASE, existing_ids, write_entry
29
  from eval.datasets import WIRED_DATASETS, get_adapter
30
 
31
  logger = logging.getLogger(__name__)
@@ -119,6 +119,7 @@ def run_predict(
119
  settings: Settings | None = None,
120
  cache_base: Path = DEFAULT_CACHE_BASE,
121
  overwrite: bool = False,
 
122
  ) -> PredictStats:
123
  """Run the pipeline over a dataset slice and cache each result.
124
 
@@ -131,13 +132,23 @@ def run_predict(
131
  cache_base: Root cache directory. Defaults to ``eval/cache``.
132
  overwrite: Re-process and overwrite examples already cached. Defaults to
133
  ``False`` so re-runs resume without re-billing.
 
 
 
134
 
135
  Returns:
136
  A :class:`PredictStats` summary of the run.
137
 
138
  Raises:
139
- ValueError: If ``dataset`` is not wired for the predict phase.
 
 
140
  """
 
 
 
 
 
141
  if dataset not in WIRED_DATASETS:
142
  wired = ", ".join(sorted(WIRED_DATASETS))
143
  raise ValueError(
@@ -146,9 +157,28 @@ def run_predict(
146
  )
147
 
148
  adapter = get_adapter(dataset)
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
149
  settings = settings or load_config()
150
  backend = create_backend(settings)
151
- already = set() if overwrite else existing_ids(cache_base, dataset)
152
 
153
  processed = skipped = accepted = review = errors = failed = 0
154
  logger.info("eval-predict: dataset=%s limit=%s backend=%s", dataset, limit, backend.name)
 
25
  from doc_agent.config import Settings, load_config
26
  from doc_agent.core import process_document
27
 
28
+ from eval.cache import DEFAULT_CACHE_BASE, errored_ids, existing_ids, write_entry
29
  from eval.datasets import WIRED_DATASETS, get_adapter
30
 
31
  logger = logging.getLogger(__name__)
 
119
  settings: Settings | None = None,
120
  cache_base: Path = DEFAULT_CACHE_BASE,
121
  overwrite: bool = False,
122
+ retry_errors: bool = False,
123
  ) -> PredictStats:
124
  """Run the pipeline over a dataset slice and cache each result.
125
 
 
132
  cache_base: Root cache directory. Defaults to ``eval/cache``.
133
  overwrite: Re-process and overwrite examples already cached. Defaults to
134
  ``False`` so re-runs resume without re-billing.
135
+ retry_errors: Re-process *only* cached entries that recorded an error,
136
+ leaving every successful prediction byte-identical. Use after an
137
+ outage. Mutually exclusive with ``overwrite``.
138
 
139
  Returns:
140
  A :class:`PredictStats` summary of the run.
141
 
142
  Raises:
143
+ ValueError: If ``dataset`` is not wired for the predict phase, if both
144
+ ``overwrite`` and ``retry_errors`` are set, or if ``retry_errors``
145
+ is set with no cache to retry from.
146
  """
147
+ if overwrite and retry_errors:
148
+ raise ValueError(
149
+ "--overwrite and --retry-errors are mutually exclusive: the first "
150
+ "re-runs every document, the second only the failed ones. Pick one."
151
+ )
152
  if dataset not in WIRED_DATASETS:
153
  wired = ", ".join(sorted(WIRED_DATASETS))
154
  raise ValueError(
 
157
  )
158
 
159
  adapter = get_adapter(dataset)
160
+
161
+ if retry_errors:
162
+ cached = existing_ids(cache_base, dataset)
163
+ if not cached:
164
+ raise ValueError(
165
+ f"--retry-errors needs an existing cache for dataset {dataset!r} "
166
+ f"under {cache_base}, but none was found. Run a normal predict first."
167
+ )
168
+ retry = errored_ids(cache_base, dataset)
169
+ # Skip everything already cached that did NOT error, so successful
170
+ # predictions are never re-run, re-billed, or rewritten.
171
+ already = cached - retry
172
+ logger.info(
173
+ "eval-predict: retry-errors mode -- %d errored of %d cached will be re-run",
174
+ len(retry),
175
+ len(cached),
176
+ )
177
+ else:
178
+ already = set() if overwrite else existing_ids(cache_base, dataset)
179
+
180
  settings = settings or load_config()
181
  backend = create_backend(settings)
 
182
 
183
  processed = skipped = accepted = review = errors = failed = 0
184
  logger.info("eval-predict: dataset=%s limit=%s backend=%s", dataset, limit, backend.name)
eval/run_eval.py CHANGED
@@ -54,6 +54,16 @@ def _build_parser() -> argparse.ArgumentParser:
54
  action="store_true",
55
  help="Re-process and overwrite already-cached examples.",
56
  )
 
 
 
 
 
 
 
 
 
 
57
 
58
  score = subparsers.add_parser("score", help="Compute metrics + sweep from the cache (offline).")
59
  _add_common(score)
@@ -103,6 +113,7 @@ def main(argv: list[str] | None = None) -> int:
103
  args.limit,
104
  cache_base=args.cache_base,
105
  overwrite=args.overwrite,
 
106
  )
107
  print(
108
  f"\nPredict complete for {stats.dataset}: "
 
54
  action="store_true",
55
  help="Re-process and overwrite already-cached examples.",
56
  )
57
+ predict.add_argument(
58
+ "--retry-errors",
59
+ action="store_true",
60
+ help=(
61
+ "Re-predict ONLY cached entries whose 'error' field is set, leaving "
62
+ "every successful prediction byte-identical. Use after a quota or "
63
+ "network outage. Unlike --overwrite this never re-runs a document "
64
+ "that already succeeded, so frozen predictions stay comparable."
65
+ ),
66
+ )
67
 
68
  score = subparsers.add_parser("score", help="Compute metrics + sweep from the cache (offline).")
69
  _add_common(score)
 
113
  args.limit,
114
  cache_base=args.cache_base,
115
  overwrite=args.overwrite,
116
+ retry_errors=args.retry_errors,
117
  )
118
  print(
119
  f"\nPredict complete for {stats.dataset}: "
tests/test_eval.py CHANGED
@@ -13,7 +13,7 @@ from typing import Any
13
 
14
  import pytest
15
 
16
- from eval.cache import read_entries, report_from_dict, write_entry
17
  from eval.metrics import (
18
  THRESHOLDS,
19
  compute_field_metrics,
@@ -22,6 +22,7 @@ from eval.metrics import (
22
  sweep_thresholds,
23
  )
24
  from eval.normalize import is_present, normalize, values_match
 
25
  from eval.score import build_report
26
 
27
 
@@ -315,3 +316,49 @@ def test_build_report_raises_without_cache(tmp_path: Path) -> None:
315
  """Scoring a dataset with no cache raises a clear error."""
316
  with pytest.raises(FileNotFoundError):
317
  build_report("missing", cache_base=tmp_path)
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
13
 
14
  import pytest
15
 
16
+ from eval.cache import errored_ids, read_entries, report_from_dict, write_entry
17
  from eval.metrics import (
18
  THRESHOLDS,
19
  compute_field_metrics,
 
22
  sweep_thresholds,
23
  )
24
  from eval.normalize import is_present, normalize, values_match
25
+ from eval.predict import run_predict
26
  from eval.score import build_report
27
 
28
 
 
316
  """Scoring a dataset with no cache raises a clear error."""
317
  with pytest.raises(FileNotFoundError):
318
  build_report("missing", cache_base=tmp_path)
319
+
320
+
321
+ # ---------------------------------------------------------------------------
322
+ # Targeted retry of failed predictions (--retry-errors)
323
+ # ---------------------------------------------------------------------------
324
+
325
+
326
+ def _cached(example_id: str, *, error: str | None) -> dict[str, Any]:
327
+ entry = _entry(
328
+ example_id,
329
+ predicted={"total": 1.0},
330
+ gold={"total": "1.00"},
331
+ confidence=0.5,
332
+ )
333
+ entry["error"] = error
334
+ return entry
335
+
336
+
337
+ def test_errored_ids_selects_only_failed_entries(tmp_path: Path) -> None:
338
+ """The retry set is exactly the entries that produced no extraction.
339
+
340
+ A document the model read and the rules then rejected is a result, not a
341
+ failure, and must never be re-run -- re-running it would replace a frozen
342
+ prediction and break any before/after rule comparison built on it.
343
+ """
344
+ for name, error in (("ok1", None), ("ok2", None), ("dead", "429 RESOURCE_EXHAUSTED")):
345
+ write_entry(tmp_path, "d", _cached(name, error=error))
346
+
347
+ assert errored_ids(tmp_path, "d") == {"dead"}
348
+
349
+
350
+ def test_errored_ids_is_empty_for_a_clean_cache(tmp_path: Path) -> None:
351
+ write_entry(tmp_path, "d", _cached("ok", error=None))
352
+ assert errored_ids(tmp_path, "d") == set()
353
+
354
+
355
+ def test_overwrite_and_retry_errors_are_mutually_exclusive(tmp_path: Path) -> None:
356
+ """Silently letting one win would re-run 361 documents when 44 were meant."""
357
+ with pytest.raises(ValueError, match="mutually exclusive"):
358
+ run_predict("sroie", 1, cache_base=tmp_path, overwrite=True, retry_errors=True)
359
+
360
+
361
+ def test_retry_errors_refuses_an_empty_cache(tmp_path: Path) -> None:
362
+ """Falling through to a full predict here would spend quota unasked."""
363
+ with pytest.raises(ValueError, match="needs an existing cache"):
364
+ run_predict("sroie", 1, cache_base=tmp_path, retry_errors=True)