fix: save online responses that parse_func rejects or filters out

[TanishGudise]

Closes #744

append_generic_response returned before writing a response when parse_func raised or returned no rows. The response was paid for but never saved, so a resumed run sent the request again, and a fixed parse_func couldn't rebuild the dataset from the cache. With the default require_all_responses=True, a parse_func that drops rows by returning [] also made every run fail with "Some requests do not have responses".

This writes the response first, then returns early only before the parsed-row count and the viewer stream. create_dataset_files already re-runs parse_func on every saved response without errors, so:

• a response parse_func raised on is counted as a failed request when the dataset is built, and after parse_func is fixed the dataset is rebuilt from the saved response without a new request;
• a row dropped with [] is skipped when the dataset is built and no longer counts as missing.

Responses that don't match response_format don't reach this write: handle_single_request_with_retries validates the format first and retries. Batch mode isn't changed here: its early return (base_batch_request_processor.py L606-L610) also covers response_format mismatches, and it already saves [] results.

Visible changes (all only when parse_func raises or returns []):

• When parse_func raises on some rows and require_all_responses=True, the error is now "Some requests failed and require_all_responses is True" instead of "Some requests do not have responses". Its error sample is empty, because create_dataset_files only samples API errors, as it already does when a changed parse_func fails on cached responses.
• When parse_func raises on every row, the error is "All requests failed" instead of "No responses files found". When it returns [] for every row, datasets raises SchemaInferenceError ("at least one example") instead. In both cases a re-run now sends 0 requests instead of all of them again.
• A request that failed after all retries is now saved with its errors even when parse_func can't handle a None response, as it already was without a parse_func. With require_all_responses=True the error then reads "Some requests failed" with a sample of the API errors. On resume these entries are removed and retried, so resume behaviour is unchanged.
• When parse_func raises, its warning is logged twice: once when the response is saved and once when the dataset is built.

Tests (tests/unittests/test_base_online_request_processor.py)

• A response that parse_func raises on, or filters out, is in completed_request_ids on resume (fails on main: {0, 2} instead of {0, 1, 2}).
• After parse_func is fixed, the dataset is rebuilt from the saved responses with every row (fails on main).
• Rows filtered out with [] don't fail the run under the default require_all_responses=True (fails on main).

Each part of the fix removed on its own makes a test fail: still dropping [] results, still dropping parse_func errors, and dropping the early return after the write.

Manual check: end to end against a local OpenAI-compatible stub, 10 rows, parse_func raising on 2: main saved 8 responses and the re-run with the fixed parse_func sent 2 requests; with this change 10 are saved and the re-run sends 0. Filtering 2 rows with default settings: main failed all 3 runs (10, 2 and 2 requests sent); with this change all 3 runs succeed with 8 rows (10, 0 and 0 requests).

Without network access, the full test suite has no new failures compared with main: the offline failures are tests that need the network, plus test_simple_code_execution_local, which also fails locally on main.

CI note: CI on main is currently red for reasons unrelated to this PR: test_basic_reasoning and test_batch_reasoning fail because litellm 1.83.7 doesn't map claude-3-7-sonnet-20250219, and total coverage is 78%, under --cov-fail-under=80 (run 37956501158 on 31abdb7). Once CI runs here (first-time contributor runs wait for approval), those failures will show on this PR too until main is fixed.

🤖 Generated with Claude Code

[strix-security]

Strix Security Review

No security issues found.

Review summary

Reviewed the production-code change in append_generic_response (reordering the JSONL cache write before the early return when parse_func fails or returns no rows) plus the accompanying unit tests. The change only alters local cache persistence and resume/dataset-rebuild behavior; it does not touch authentication, authorization, input handling, cryptography, file paths, network requests, or any sink reachable by untrusted input. No security-relevant vulnerability was found in the changed code.

_{Updated for ad23dd6.}

---

Reviewed by Strix
_{Re-run review · Configure security review settings}

[entelligence-ai-pr-reviews]

EntelligenceAI PR Summary

Updated append_generic_response to persist every API response before handling parse results, including responses where parse_func raises or returns no rows. This prevents paid requests from being retried on resume, allows datasets to be rebuilt after fixing a parser, and avoids failures when rows are intentionally filtered out. Added tests covering parser exceptions, filtered rows, cache-based dataset rebuilding, and completed-request detection.

flowchart TD
    classDef newBehavior fill:#dcfce7,stroke:#16a34a,color:#14532d;
    A["API response received"] --> B["parse_func processes response"]
    B --> C["Persist raw and parsed response"]
    C --> D["Resume skips completed request"]
    C --> E["Dataset rebuild uses cached response"]
    B --> F["No parsed rows or parse error"]
    F --> C
    class B,C,D,E,F newBehavior;

Loading

🟢 Green = new or changed in this PR

---

Review Scorecard

Dimension	Rating	Basis
Code Quality	●●●●● 5/5 — Excellent	no critical/significant findings — rated Excellent
Blast Radius	Low	changed symbols are referenced only within their own file(s); no high-impact surface touched, 1 file(s) / ~6 line(s) changed (size only — not a blast signal)
Merge Confidence	●●●●● 5/5 — Safe to Merge	code quality 5/5 × Low blast radius

Safe to merge with a low blast radius; code quality rated Excellent (5/5): the review found nothing to flag.

Key Findings:

• No critical or significant issues found

Evaluated against

• 2/2 changed files reviewed
• criteria: correctness, security & access control, robustness & error handling, concurrency & data integrity, repo conventions / steering docs
• steering docs: none found in repo

[entelligence-ai-pr-reviews]

Walkthrough

This update strengthens online response caching and resume behavior by persisting every processed response, including parse failures and empty results. It also expands unit coverage to verify skipped re-requests, completion tracking, dataset reconstruction from cached responses, and exclusion of filtered rows.

Changes

Files

• src/bespokelabs/curator/request_processor/online/base_online_request_processor.py

Updated append_generic_response to serialize and persist responses before returning when parsing fails or produces no rows. Non-empty responses retain their existing parsing-counter and downstream-metadata updates, while all processed responses are now available for resumed runs and dataset reconstruction.

Files

• tests/unittests/test_base_online_request_processor.py

Expanded unit tests with lightweight processor fixtures and response helpers covering caching, resume behavior, parse exceptions, empty parse results, completion tracking, dataset rebuilding after parser fixes, and filtering without run failure.

[TanishGudise]

Closing as superseded by #754, which fixes #744 more completely. I ran this PR's regression tests against main after #754 and all four pass. Thanks for the quick fix.