refactor: batch recovery enums, registry index, and parameter cleanup#630
Merged
Conversation
- Replace O(n) list().index() in BatchResultReconciler.get_record_index() with dict lookup built at init time - Remove exists() guard before read_text() in _merge_carry_forward() to eliminate TOCTOU race; catch FileNotFoundError instead - Make _validate_config() return compiled schema so prepare_tasks() avoids duplicate _prepare_schema() call (saves file I/O + compilation)
Replace bare "retry"/"reprompt"/"done" strings with RecoveryType and RecoveryPhase enums in batch_constants.py. Update BatchJobEntry, RecoveryState, processing.py, and processing_recovery.py to use enums. Both enums inherit str so JSON round-trips are transparent — existing recovery state files on disk deserialize without breaking.
Replace O(n) linear scans in get_batch_job_by_id() and update_status() with O(1) dict lookups via _batch_id_index (batch_id → file_name). Index is rebuilt under the existing lock on every cache mutation.
Add RecoveryContext (service/manager/provider/agent_config/output_directory/ action_name/start_time) and BatchIdentity (batch_id/file_name/entry) dataclasses. Refactor handle_retry_recovery, handle_reprompt_recovery, check_and_submit_reprompt, finalize_batch_output, and _finalize_and_cleanup from 11-13 positional params down to 5-6 using the new context objects.
Deduplicate BatchContextAdapter.to_processing_context() calls from the three passthrough builders (_build_exhausted_passthrough, _build_unprocessed_passthrough, _build_failed_passthrough) into a shared _attach_passthrough_context helper.
Update all test files that directly call handle_retry_recovery, handle_reprompt_recovery, finalize_batch_output, check_and_submit_reprompt, and cleanup_recovery to use the new RecoveryContext + BatchIdentity parameter objects. No test logic changed.
- Use BatchStatus.SUBMITTED enum instead of "submitted" string literal in _remove_batch_placeholder - Use lazy index invalidation in BatchRegistryManager — invalidate on mutation, rebuild on first lookup — avoids O(n) rebuild on every save/remove when lookups may not follow
Muizzkolapo
force-pushed
the
refactor/batch-recovery-cleanup
branch
from
7239dfe to
b7bdd79
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
BatchResultReconciler.get_record_index()andBatchRegistryManager.get_batch_job_by_id()/update_status()with dict-based index lookups"retry"/"reprompt"/"done"strings withstrenums (JSON round-trip transparent — existing recovery state files on disk load without breaking)_merge_carry_forward(), deduplicate schema compilation inBatchTaskPreparator, extract_attach_passthrough_contexthelper from 3 copy-pasted passthrough builders15 files changed, 7311 tests pass, 2 skipped, ruff clean.
Test plan
ruff checkandruff format --checkcleanlist().index()in reconcilercarry_path.exists()TOCTOU guard_prepare_schemacalled only twice (definition + inside_validate_config)"retry"/"reprompt"strings outside enum definitionsget_batch_job_by_id/update_statusBatchContextAdapter.to_processing_contextcalled only 2x in batch_result_strategy (successful + helper)