Skip to content

Improve extract.ai retries, model validation, and OpenAI logs - #1171

Merged
ebhills merged 6 commits into
mainfrom
codex/fix-extract-ai-transport-retries
Sep 8, 2026
Merged

Improve extract.ai retries, model validation, and OpenAI logs#1171
ebhills merged 6 commits into
mainfrom
codex/fix-extract-ai-transport-retries

Conversation

@ebhills

@ebhills ebhills commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

This fixes extract.ai retries that could exhaust a shared deadline or mishandle failed HTTP responses, stops repeated submissions when OpenAI reports an invalid model, and makes Responses logs available by default with recipe/user attribution.

Linked issue

Closes #1170

What changes

  • Timeouts and retries: Remove the shared 15-second deadline from extraction, OpenAI transports, and cache coordination. Each HTTP attempt receives the full configured timeout; queued rows and retry waits no longer consume a shared budget. Responses now correctly distinguishes transport failures from HTTP error responses and retries eligible failures.
  • Concurrency: Rename the configuration setting to extract_ai.default_concurrency, defaulting to 32. An explicit threads value remains the per-call override.
  • Invalid models: Complete the first row/unique request before submitting the remainder. If OpenAI returns model_not_found, raise once and stop the wrangle instead of submitting the same invalid request for every row. This applies to Responses and Chat Completions, with caching enabled or disabled; the first result can still come from cache.
  • Response storage: Default store to true in both packaged configuration and the missing-setting fallback, enabling inspection in OpenAI Logs > Responses. Per-call store: false overrides the default.
  • Metadata: Forward a validated metadata object on both API protocols. Add available recipe_name and wrangles_user labels automatically. Saved recipes use their title, local files use their basename, and inline callers can supply a recipe_name run variable. User attribution uses WRANGLES_USER, XL's user_email, or the configured Wrangles login. Explicit labels override defaults; metadata: {} disables automatic labels. Validation enforces OpenAI's 16-pair, 64-character key, and 512-character value limits.
  • Context and caching: Preserve attribution through nested recipes and threaded batches, isolate concurrent runs, and restore context after errors without rewriting recipe variables. Effective metadata and response-storage settings are included in local result-cache identity. Cache hits do not create another OpenAI request/log; changing labels or storage separates cached results. Metadata remains separate from model instructions and does not change the OpenAI prompt-cache key.
  • Update the recipe schema, configuration guidance, notebook examples, and regression tests.
  • Test dependencies: Pin jsonschema==4.26.0 in all five CI/release pytest dependency installations, including container and macOS jobs. This fixes the metadata test's missing-import failure during collection without adding a package runtime dependency.

How it was verified

  • Local offline validation: 170 tests passed on Python 3.13.1 for the code committed as 9407a09f. OpenAI requests were mocked; coverage includes retries, invalid-model handling, storage overrides, metadata limits/forwarding, cache separation, and concurrent/nested context cleanup.

    python -m pytest tests/test_openai_extract_ai.py tests/test_extract_ai_metadata.py tests/test_extract_ai_metadata_context.py tests/test_ai_cache.py tests/test_ai_definition.py tests/recipes/wrangles/test_main.py::TestWrangleSchema::test_extract_ai_schema_documents_public_parameters_and_example_shape -q
    
  • git diff --check passed.

  • Latest GitHub CI at 2ed1ec0c is running: run 34287988288 has passed configuration, schema generation, and the Ubuntu/Python 3.11 pip-install check. The pytest matrix is now in progress; the complete CI result is pending. The prior 9407a09f run stopped during test collection because jsonschema was missing. Commit 2ed1ec0c adds that test dependency; the new CI result is still pending.

  • No live OpenAI, deployed Lambda, or production Excel verification is claimed for the metadata changes. The separate XL companion passed four mocked Jest tests; those are outside this Python PR.

Compatibility and risk

  • Remove the former deadline argument and total_deadline_seconds configuration entry. Replacement configuration files must rename max_concurrency to default_concurrency; the old key is no longer read. Use timeout, retries, and threads for call controls.
  • A batch can now run longer than 15 seconds, and the initial request completes before parallel fan-out. WranglesXL and other hosts still impose their own execution limits; this PR does not extend those limits.
  • Responses inputs and outputs are now eligible for storage at OpenAI by default. Set store: false where storage should be disabled, and independently use metadata: {} to disable automatic labels. Labels are diagnostic attribution, not authenticated audit identities. Only the selected labels are added automatically; metadata attribution adds no authentication or metadata-service lookup.
  • Automatic displayed recipe names from WranglesXL require the separate XL companion update, which passes the title as a run variable. That change is outside this PR and must be published/deployed separately; older clients can supply the variable or explicit step metadata.
  • The store option applies to Responses and is not forwarded as Chat Completions storage. Agents SDK workflow tracing remains separate, with a follow-up issue planned after this work is complete.
  • Immediate configuration rollback for the logging behavior is store: false plus metadata: {}. Reverting the PR or restoring the prior package is required to restore the former deadline/concurrency behavior.
  • Before requesting review, confirm the latest CI run completes successfully. Live log attribution should be verified after both runtime and XL updates are deployed.

Ready-for-review checklist

  • One human delivery owner is assigned
  • The linked issue and intended milestone are correct
  • The branch is current with main and has no merge conflicts
  • Focused tests pass
  • New or changed behavior has direct test coverage
  • Documentation/schema/configuration is updated where applicable
  • The PR contains no unrelated changes
  • The PR description reflects the branch's current scope and latest validation
  • One primary reviewer is requested only when this PR is ready

See the pull request workflow.

This drops the shared `deadline` behavior from `extract.ai`, OpenAI transport calls, and AI cache coordination so retries always use the full configured per-attempt timeout. It also renames the default concurrency setting from `max_concurrency` to `default_concurrency`, keeps `threads` as the per-call override, and updates recipe schema/docs to reflect the new timeout/retry semantics. Tests were expanded and adjusted to cover concurrency resolution, transport retry behavior, long retry delays, queued row processing over longer runtimes, and the removal of deadline-specific validation and outcomes.
@ebhills ebhills self-assigned this Sep 8, 2026
@ebhills
ebhills requested a review from thomasstvr September 8, 2026 20:15
Changed 'store' setting to true to enable logging at OpenAI.
@ebhills

ebhills commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator Author

@copilot - when I created this PR, the initial CI failed several extract.ai tests. Please review and fix those.

One of the failing tests did not make sense - it was trying to print an error message for every row when it was pointed at an invalid OpenAI model. We use the same model for all rows so there is no need to send repetitive messages. In fact, perhaps we should change the code to check model validity on the first row of the first batch or as a pre-flight check, and if it's invalid fail the recipe at that point without any further submissions.

thomasstvr
thomasstvr previously approved these changes Sep 8, 2026
Co-authored-by: ebhills <53243273+ebhills@users.noreply.github.com>
Co-authored-by: ebhills <53243273+ebhills@users.noreply.github.com>

Copilot AI commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

@copilot - when I created this PR, the initial CI failed several extract.ai tests. Please review and fix those.

One of the failing tests did not make sense - it was trying to p...

Implemented in 5d9357d with the fallback correction in 7c03dd1. extract.ai now validates the first unique row before submitting the remainder and raises once on OpenAI model_not_found; regression coverage spans Responses/Chat Completions and cache on/off. The stale live test now expects recipe failure rather than per-row HTTP 400 strings. Checks: 77 focused tests and 1,601 credential-safe tests passed; CodeQL found 0 alerts. The latest CI run is action_required with no jobs started.

Recommended disposition: Comment only

Next steps

  1. PR assignee: Approve the pending CI workflow for 7c03dd1.
  2. Reviewer: After CI passes, verify the fail-fast behavior, resolve this conversation, and submit a fresh approval.

Introduces `metadata` support for `extract.ai`, including validation against OpenAI limits, forwarding on both Responses and Chat Completions paths, and automatic diagnostic labels (`recipe_name`, `wrangles_user`) sourced from recipe run context. Recipe context tracking now carries and restores attribution across nested and concurrent runs, with saved/file recipe name detection and XL variable precedence handling. The result-cache identity now includes response storage and effective metadata so attribution/storage variants do not share cached results. Documentation and schema docs were updated for `store` behavior, log visibility, label overrides, and cache implications, and comprehensive offline tests were added for metadata validation, forwarding, context behavior, and cache separation.
@ebhills ebhills changed the title Remove extract.ai deadline budget logic Improve extract.ai retries, model validation, and OpenAI logs Sep 8, 2026
Add `jsonschema==4.26.0` to dependency installation steps in `ci.yml` and `publish-tagged.yml` (both full dependency and test-only installs). This keeps GitHub Actions runs consistent and avoids failures from unpinned `jsonschema` version changes.
@ebhills
ebhills merged commit 714008b into main Sep 8, 2026
13 checks passed
@ebhills
ebhills deleted the codex/fix-extract-ai-transport-retries branch September 8, 2026 23:35
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.

Undefined OpenAI Error in new extract.ai

3 participants