UN-3636 [DEV] Hermetic LLM mock hook for execute-path critical tests#2170
UN-3636 [DEV] Hermetic LLM mock hook for execute-path critical tests#2170chandrasekharan-zipstack wants to merge 17 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe change adds deterministic LLM mocking, shared hermetic E2E provisioning, API deployment and ETL execution tests, expanded critical-path coverage, CI image caching, and dependency-aware test-group blocking and reporting. ChangesHermetic E2E coverage
Dependency-aware group execution
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant E2ETests
participant ProvisionedWorkflow
participant API
participant Workers
participant LLM
E2ETests->>ProvisionedWorkflow: provision mocked workflow
ProvisionedWorkflow->>API: create and configure workflow
E2ETests->>API: dispatch execution
API->>Workers: process execution
Workers->>LLM: request completion with mock_response
LLM-->>Workers: return canned answer
Workers-->>API: update execution result
API-->>E2ETests: deliver completed result
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Execute-path critical paths (workflow-create-execute, usage-token-tracking, etc.) need a deterministic LLM without real provider secrets. Add a single sdk1 escape hatch: when UNSTRACT_LLM_MOCK_RESPONSE is set, inject litellm's mock_response so completions return that string with fixed usage (10/20/30), tunable via DEFAULT_MOCK_RESPONSE_* and error sentinels like "litellm.RateLimitError". No-op in production (env unset). Wire the env through worker-executor-v2 / worker-file-processing-v2 in the e2e compose overlay so the mock reaches the process that runs the LLM. Covered by unit tests pinning both our injector and the litellm mock contract (string + deterministic usage + error sentinel) so a litellm bump can't silently break hermetic execute-path coverage. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017UVfw7aAocC3KCJaEyZ5GU
f26d44c to
fc0d056
Compare
Add e2e tests that exercise the full execute path hermetically using the UNSTRACT_LLM_MOCK_RESPONSE hook (no real LLM/secret): - api-deployment-run + usage-token-tracking: deploy a workflow as an API, POST a doc synchronously, assert the mocked answer and fixed 10/20/30 usage come back as structured JSON. - workflow-create-execute: two-step /workflow/execute/ then poll to COMPLETED; COMPLETED + a successful file is the hermetic proof (without the mock the completion would fail). A shared provisioned_workflow fixture stands up a Prompt Studio-backed API workflow (NoOp x2text/vectordb, chunk_size=0 so embedding/vectordb are skipped). The rig seeds a canonical mock string in ComposeRuntime so both the workers and pytest see the same value; tests skip if it is unset. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017UVfw7aAocC3KCJaEyZ5GU
- test_mock_response.py: split the module docstring summary line (D205 was failing pre-commit.ci, the only red hook on the PR). - groups.yaml: drop `optional: true` from e2e-workflow / e2e-api-deployment. They now hold real execute-path tests rather than placeholders, so a red one should fail the build directly instead of only surfacing as a critical-path gap via --fail-on-critical-gap. - README: the workflows/ and api_deployment/ dirs are no longer "(future)", and document UNSTRACT_LLM_MOCK_RESPONSE under E2E runtime — including that only completions are mocked (embedding.py is unhooked; chunk_size=0 is what keeps indexing off the real provider). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P4tU5vyB4G6BgfAnGqjxLs
The comment block sat where #2176 reworded the e2e-login comment and inserted a new group, creating a textual merge conflict against main. Move it inside each group body; the optional: true removals merge cleanly on their own. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P4tU5vyB4G6BgfAnGqjxLs
Drop the restated-critical-path and mechanism-narrating comments across the LLM mock and the execute-path e2e tests; keep only the constraints the code can't show (litellm's fixed usage, the mandatory filter arg, the two-call execute contract). Also drop the groups.yaml 'not optional' comment: the field is self-evident and would go stale. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P4tU5vyB4G6BgfAnGqjxLs
Three follow-ups from review: - llm.py: warn once per process when UNSTRACT_LLM_MOCK_RESPONSE is active. The hatch was silent, so a stray env var in production would fake every completion and its billing with nothing in the logs. - rig: implement the runtime-gate-skip TODO in cmd_run. Groups run in topo order, so a dep's result is known before its dependents start; a red dep means the precondition it gates does not hold and dependents would only produce noise against a half-up stack. Adds GroupManifest.transitive_deps and a 'blocked' GroupResult status which is never green, so a blocked group attests no coverage and --fail-on-critical-gap still catches a path that quietly stopped being covered. This mattered less while every platform dependent was optional; it does now that the execute-path groups gate. - api-deployment e2e: the 10/20/30 usage assert now explains itself, since counts are summed per reason and a multiple means extra completions rather than broken tracking. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P4tU5vyB4G6BgfAnGqjxLs
Closes the remaining gaps in the registry; every path is now proof: marker. - callback-result-delivery: async API execution (timeout=0). With a file to process, COMPLETED is written only by the chord callback, so a polled result proves the callback worker ran. Pins the one-shot read too — fetching a delivered result acknowledges it and the next read 406s, which a retry loop added later would otherwise swallow silently. - workflow-execution-fan-out: multi-file execution, one batch per file. Needed MAX_PARALLEL_FILE_BATCHES on the *backend* (workers ask it for the value and only fall back to their own env); at the default of 1 the files share a batch and run serially, so the test would have passed proving nothing. File bodies differ because identical ones are deduplicated by hash on ingest. - prompt-studio-fetch-response: the authoring surface. Async, so the answer is polled from prompt-output/ rather than the task status, which reports the executor finishing and not the callback having stored anything. Upload is content-sniffed and OSS takes PDF only, hence the real (if empty) PDF. - pipeline-etl-execute: source connector to destination, over MinIO — the only storage connector the compose stack both boots and registers. Reuses the exported tool but needs its own workflow, since endpoints are per-workflow and the existing one is committed to the API deployment tests. None of these have run: the e2e job is skipped while the PR is a draft. Locally verified only as far as it goes here — rig validate, rig self-tests, ruff, and pytest collection. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P4tU5vyB4G6BgfAnGqjxLs
|
| Filename | Overview |
|---|---|
| unstract/sdk1/src/unstract/sdk1/llm.py | Adds the env-controlled LiteLLM mock hook at the completion call sites. |
| tests/rig/cli.py | Adds dependency blocking, blocked-result persistence, and mock env setup for platform test runs. |
| tests/rig/reporting.py | Adds blocked status handling in reports, summaries, and junit parsing. |
| tests/e2e/etl/conftest.py | Adds MinIO-backed ETL workflow setup for source and destination endpoints. |
| tests/e2e/etl/test_pipeline_etl_execute.py | Adds the ETL critical-path test that seeds MinIO, runs a pipeline, and checks destination output. |
Reviews (9): Last reviewed commit: "UN-3636 [FIX] Keep frontend in e2e scope" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…ll minio
The e2e tier failed on two counts:
- api-deployment run/fan-out asserted COMPLETED on the synchronous POST, but a
file-bearing execution only reaches COMPLETED once the chord callback rejoins
— the sync path reports EXECUTING under CI load. All three api-deployment
tests now dispatch async (timeout=0) and poll the status endpoint via shared
dispatch_async/poll_delivered helpers, tolerating transient poll timeouts
(the async test's earlier ReadTimeout). execution_id is parsed from status_api
since the async PENDING body omits it as a top-level field.
- e2e-etl skipped ("minio client not installed"), leaving pipeline-etl-execute a
gap. Add minio to the rig's injected pytest deps so the test runs.
Verified against a full local stack: api_deployment (3), workflow, etl,
prompt_studio all green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017UVfw7aAocC3KCJaEyZ5GU
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Clears the SonarCloud S5332 clear-text-HTTP finding on the test fixture (compose-internal MinIO has no TLS) by sourcing the scheme from env, and trims the low-value minio dependency comment. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017UVfw7aAocC3KCJaEyZ5GU
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
tests/e2e/api_deployment/conftest.py (1)
84-96: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo backoff on
ConnectionError/Timeoutretries.
except _TRANSIENT: continue(Lines 89-90) retries immediately with no sleep, unlike the other branches. ATimeoutis naturally throttled by the 30s request timeout, but a fast-failingConnectionError(e.g., connection reset) would busy-loop against the status endpoint for up to_POLL_TIMEOUT_SECONDS.Proposed fix
except _TRANSIENT: + time.sleep(2) continue🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/e2e/api_deployment/conftest.py` around lines 84 - 96, Update the polling retry logic around the transient exception handler in the status-wait function to sleep for the existing polling interval before continuing. Preserve the current handling for successful responses and non-transient failures, while ensuring fast-failing ConnectionError retries do not busy-loop.tests/e2e/conftest.py (1)
116-131: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCSRF-wrapping
_post/_patchhelpers are reimplemented per file. Both sites attachX-CSRFTokenfrom the session cookie and POST/PATCH with a fixed timeout — one root cause (no shared e2e HTTP helper), duplicated per file.
tests/e2e/conftest.py#L116-L131: keep as the canonical_post/_patch(accepting arequests.Session); consider exporting them for reuse instead of re-deriving per-module wrappers.tests/e2e/prompt_studio/test_prompt_studio_fetch_response.py#L101-L103: replace the local_postwith a thin call intotests/e2e/conftest.py's_post(pw.session, ...)instead of reimplementing the CSRF-attach logic.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/e2e/conftest.py` around lines 116 - 131, The CSRF-aware HTTP helpers are duplicated across e2e files. Keep tests/e2e/conftest.py lines 116-131 as the canonical _post and _patch implementations, making them reusable if needed; in tests/e2e/prompt_studio/test_prompt_studio_fetch_response.py lines 101-103, remove the local _post wrapper and delegate to conftest.py’s _post using pw.session, preserving the existing request arguments and timeout behavior.tests/rig/cli.py (1)
449-457: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting the failure predicate to avoid 3-way drift.
"Did this group fail" is now computed independently in three places with slightly different
optionalgating (cascade tracking at Line 478-481 ignoresoptional; the twooverall_exitfolds at Line 493-498 and 502-508 respect it). A future tweak to failure criteria (e.g. new non-failing exit code) risks updating only some sites.♻️ Suggested consolidation
+def _group_failed(exit_code: int, result: GroupResult | None) -> bool: + return exit_code not in _NON_FAILING_PYTEST_EXIT_CODES or ( + result is not None and (result.errors or result.failed) + ) + if result is not None: group_results.append(result) # Tracked regardless of `optional` — see the cascade note above. - if exit_code not in _NON_FAILING_PYTEST_EXIT_CODES or ( - result is not None and (result.errors or result.failed) - ): + if _group_failed(exit_code, result): failed_groups.add(name) ... - if ( - exit_code not in _NON_FAILING_PYTEST_EXIT_CODES - and not group.optional - and overall_exit == 0 - ): + if _group_failed(exit_code, None) and not group.optional and overall_exit == 0: overall_exit = exit_code - if ( - result is not None - and (result.errors or result.failed) - and not group.optional - and overall_exit == 0 - ): + if _group_failed(0, result) and not group.optional and overall_exit == 0: overall_exit = 1Also applies to: 477-481, 493-508
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/rig/cli.py` around lines 449 - 457, Extract a shared failure predicate for group results and use it consistently in the runnable-group cascade tracking and both overall_exit folds. Ensure the predicate applies the same optional-group gating everywhere, replacing the duplicated exit-code checks while preserving blocked-result handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/e2e/conftest.py`:
- Around line 250-263: Track whether the workflow endpoint setup loop in the
fixture successfully patches at least one matching endpoint to API, and assert
that this occurred after iterating through eps. Keep the existing matching and
patch status assertion, but fail immediately with a clear fixture-level message
when eps is empty or no endpoint matches workflow_id.
---
Nitpick comments:
In `@tests/e2e/api_deployment/conftest.py`:
- Around line 84-96: Update the polling retry logic around the transient
exception handler in the status-wait function to sleep for the existing polling
interval before continuing. Preserve the current handling for successful
responses and non-transient failures, while ensuring fast-failing
ConnectionError retries do not busy-loop.
In `@tests/e2e/conftest.py`:
- Around line 116-131: The CSRF-aware HTTP helpers are duplicated across e2e
files. Keep tests/e2e/conftest.py lines 116-131 as the canonical _post and
_patch implementations, making them reusable if needed; in
tests/e2e/prompt_studio/test_prompt_studio_fetch_response.py lines 101-103,
remove the local _post wrapper and delegate to conftest.py’s _post using
pw.session, preserving the existing request arguments and timeout behavior.
In `@tests/rig/cli.py`:
- Around line 449-457: Extract a shared failure predicate for group results and
use it consistently in the runnable-group cascade tracking and both overall_exit
folds. Ensure the predicate applies the same optional-group gating everywhere,
replacing the duplicated exit-code checks while preserving blocked-result
handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 13ad4ba2-a18b-4cb3-b034-91f3430b5f52
📒 Files selected for processing (25)
tests/README.mdtests/compose/docker-compose.test.yamltests/critical_paths.yamltests/e2e/api_deployment/__init__.pytests/e2e/api_deployment/conftest.pytests/e2e/api_deployment/test_api_deployment_async.pytests/e2e/api_deployment/test_api_deployment_fan_out.pytests/e2e/api_deployment/test_api_deployment_run.pytests/e2e/conftest.pytests/e2e/etl/__init__.pytests/e2e/etl/conftest.pytests/e2e/etl/test_pipeline_etl_execute.pytests/e2e/prompt_studio/__init__.pytests/e2e/prompt_studio/test_prompt_studio_fetch_response.pytests/e2e/workflows/__init__.pytests/e2e/workflows/test_workflow_execute.pytests/groups.yamltests/rig/cli.pytests/rig/groups.pytests/rig/reporting.pytests/rig/runtime.pytests/rig/tests/test_groups.pytests/rig/tests/test_reporting.pyunstract/sdk1/src/unstract/sdk1/llm.pyunstract/sdk1/tests/test_mock_response.py
Assert at least one endpoint was patched to API in the provisioning fixture, so a field-name mismatch surfaces here instead of as a confusing downstream execute failure (CodeRabbit). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017UVfw7aAocC3KCJaEyZ5GU
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017UVfw7aAocC3KCJaEyZ5GU
The e2e job runs on ephemeral GitHub-hosted runners, so a plain `docker compose build` rebuilt every image cold each run (~3min). Switch to docker/bake-action with per-service type=gha cache scopes matching the release workflow, so each runner restores layers from the shared GHA cache (including entries the release build already wrote). load=true exports the images into the daemon for the compose stack to boot. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017UVfw7aAocC3KCJaEyZ5GU
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/ci-test.yaml (1)
171-178: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valuePin GitHub Actions to specific commit SHAs.
The newly added
setup-buildx-actionandbake-actionuse mutable version tags (@v4and@v7). Consider pinning them to specific commit SHAs to maintain consistency with thecheckout,login-action, andsetup-uvsteps earlier in this workflow and to strengthen supply chain security against upstream tag hijacking.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci-test.yaml around lines 171 - 178, Pin the newly added docker/setup-buildx-action and docker/bake-action steps to immutable commit SHAs instead of the mutable v4 and v7 tags, preserving their current action versions and configuration.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/ci-test.yaml:
- Around line 171-178: Pin the newly added docker/setup-buildx-action and
docker/bake-action steps to immutable commit SHAs instead of the mutable v4 and
v7 tags, preserving their current action versions and configuration.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d4e93703-fb7d-4774-aa7c-c19dd594ce6a
📒 Files selected for processing (1)
.github/workflows/ci-test.yaml
SonarCloud flagged the two floating action tags added for the e2e gha cache as new MAJOR vulnerabilities (security rating C). Pin to the commit SHA the tag points to so a moved tag can't inject code. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017UVfw7aAocC3KCJaEyZ5GU
e2e ran on every non-draft PR regardless of paths, so image-only or docs-only changes triggered a full ~7min platform boot for nothing. Add an `e2e_relevant` path scope that skips frontend/** and docs/** while keeping docker/** in scope (image changes must still be e2e-tested), gate the e2e job on it, and accept a skipped e2e as a pass in the report gate (matching how a skipped test tier is handled). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L549habQ6K2m1ACJ7DwtsE
ritwik-g
left a comment
ritwik-g
left a comment
There was a problem hiding this comment.
Automated review — UN-3636 execute-path e2e
Verified against the actual head commit, the live backend source, and the PR's own green CI run (29740023152): all six new e2e tests genuinely executed and passed there (e2e-workflow 1, e2e-prompt-studio 1, e2e-etl 1, e2e-api-deployment 3), and all seven test_mock_response.py tests pass with no provider key. That evidence retires a lot of speculation — bake builds and loads correctly, e2e_relevant does evaluate true, the mock reaches the workers, the adapter/connector UUIDs resolve, and the critical-path markers all map to real tests.
Findings below are what survived that verification. Nothing here disputes the approach — the injection-site hook is the right shape, and removing optional: true plus wiring dep-failure cascade genuinely strengthens the gate.
One P1, in the CI gate rather than the test code.
Note on the PR description (not a code finding)
The description is stale and now contradicts the diff. It still says under "Not in this PR" that the execute-path e2e tests "will land next" and that "critical-path mappings are intentionally left unflipped until a real passing test exists" — but this PR contains exactly those tests and flips six mappings in critical_paths.yaml. Worth rewriting before merge so the merge commit describes what actually shipped.
PR title conforms to TICKET [TYPE] TITLE.
Posted as COMMENT, not an approval.
| e2e_result="${{ needs.e2e.result }}" | ||
| [ "$test_result" = "success" ] || [ "$test_result" = "skipped" ] || exit 1 | ||
| [ "$e2e_result" = "success" ] || exit 1 | ||
| [ "$e2e_result" = "success" ] || [ "$e2e_result" = "skipped" ] || exit 1 |
There was a problem hiding this comment.
P1 — a failed changes job now turns the required check green with zero tests run. [verified: read the full file; needs.changes appears only at :60 and :128, needs.changes.result is never checked anywhere]
This PR adds needs: changes to e2e (:128). GitHub sets a job's result to skipped when an upstream needs job fails or is cancelled. So if changes fails for any reason — paths-filter error, checkout failure, runner death — then:
needs.test.result==skipped→ passes line 343needs.e2e.result==skipped→ passes line 344reportstill runs (if: always(), :129) and the single branch-protection check reports success
with not one test having executed.
This is a regression introduced here. Before this PR e2e had no needs: at all, so a broken changes job still left e2e running and [ "$e2e_result" = "success" ] || exit 1 caught it. The comment that guarded exactly this ("e2e always runs, so it must be green") was removed in the same hunk, and changes is in report's needs list (:125) but its result is never consulted.
One line fixes it:
[ "${{ needs.changes.result }}" = "success" ] || exit 1Related, unverified: workflow_dispatch is a declared trigger (:20). If paths-filter resolves its base to the default branch, a manual run on main diffs main-against-main → e2e_relevant=false → e2e skipped → green via the same path. Pre-existing for test, but this PR newly extends it to e2e. Worth confirming.
| resp = deployment.session.get( | ||
| status_url, headers=deployment.auth, params=params, timeout=30 | ||
| ) | ||
| except _TRANSIENT: |
There was a problem hiding this comment.
P2 — the transient-retry path is unsound against this specific endpoint, and can convert a network blip into a hard test failure. [verified: read backend/api_v2/api_deployment_views.py get() :165-237 and backend/workflow_manager/workflow_v2/workflow_helper.py :401-431]
Three problems, one root cause — the retry was written without the endpoint's real status-code surface or its acknowledge-on-read semantics.
1. A lost 200 makes the retry fail the test. In workflow_helper.py, result_acknowledged is snapshotted at :403 before _set_result_acknowledge(execution) runs at :409, and that call also does ResultCacheUtils.delete_api_results(...). The server therefore acknowledges and deletes the result before the response leaves. If that response is lost in transit — precisely the Timeout/ConnectionError this handler exists to tolerate — the retry gets 406, and line 93's assert resp.status_code == 422 fails with poll: HTTP 406. The retry is unsafe against the one endpoint it retries. The async test's own comment (test_api_deployment_async.py:41-43) documents this destructive read, so the hazard is already known — it just isn't handled here.
2. assert resp.status_code == 422 doesn't match the contract. The view can return 400 (:174), 500 (:208, contains_tool_not_found_error), 406 (:193), 422 (:218/:221) and 200 (:223). A tool-not-found deployment state returns 500, and the loop dies on poll: HTTP 500 — presented as a poll-protocol violation rather than the real diagnosis, which the 500 body actually contains.
3. continue at :90 has no sleep. The time.sleep(2) at :96 is inside the 422 branch only. A ConnectionError from a refused connection returns in microseconds (timeout=30 is a connect/read timeout, not a backoff), so a down backend produces an unthrottled request loop for the full 300s. And because last is only assigned at :94, the terminal message becomes result never delivered within 300s; last: — five minutes of spinning reported with zero evidence, since the exception is never captured.
Suggested shape: sleep on every iteration, capture the exception into last, treat 406 as "already delivered" rather than an assert, and let non-{200,406,422} surface as a real error rather than a poll assert.
| - "!docs/**" | ||
| e2e_relevant: | ||
| - "!frontend/**" | ||
| - "!docs/**" |
There was a problem hiding this comment.
P2 — frontend-only PRs now lose all image-build coverage. [verified: read ci-frontend-lint.yaml and docker/dockerfiles/frontend.Dockerfile]
e2e_relevant excludes frontend/**, so a frontend-only PR skips e2e entirely. The only other workflow triggered on frontend/** is ci-frontend-lint.yaml, which runs biome ci src/ — a linter. It never builds.
Before this PR, e2e ran on every non-draft PR and built the frontend image, which does real work:
frontend.Dockerfile:44—RUN bun install --frozen-lockfile --ignore-scriptsfrontend.Dockerfile:50—RUN bun run build
So a frontend-only change that breaks bun run build (bad import, TS/bundler error) or desyncs bun.lock from package.json is lint-clean and now merges green — it will next be caught by production-build.yaml on main, i.e. after merge.
The cost saving is real and worth having; the gap is that nothing replaces the build. Either keep frontend/** in e2e_relevant, or add a build-only job to the frontend lint workflow.
| # gates (e.g. e2e-smoke: is the platform actually up?) does not hold, so | ||
| # running its dependents only yields noise against a half-up stack. | ||
| # | ||
| # `optional` cascades like any other dep: it governs whether a failure |
There was a problem hiding this comment.
P2 — groups.py's docstring now asserts the opposite of what this PR implements, and points at a TODO this PR deletes. [verified: read tests/rig/groups.py:222-228 in this worktree]
_validate_platform_groups_depend_on_gate's docstring still reads:
Note: runtime skip-on-gate-failure (not running dependents when the gate reports red) is NOT implemented yet —
cmd_runexecutes every runnable group unconditionally. Today this is moot because every dependent isoptional: true(a placeholder) [...] add dep-failure tracking incmd_run[...] see the TODO there.
All three claims are now false: this hunk implements the tracking, groups.yaml removes optional: true from e2e-workflow / e2e-api-deployment / e2e-prompt-studio, and the referenced TODO is gone — so the cross-reference dangles.
It's the docstring of the function that enforces the structural half of this invariant, so the next person reading it will conclude there is no runtime gating when there now is. Worth updating in the same PR that invalidates it.
| "UNSTRACT_MINIO_INTERNAL_URL", f"{scheme}://unstract-minio:9000" | ||
| ) | ||
| client = minio.Minio( | ||
| endpoint, access_key=access_key, secret_key=secret_key, secure=False |
There was a problem hiding this comment.
P3 — UNSTRACT_MINIO_SCHEME can't do what its comment says; the SonarCloud S5332 fix is cosmetic. [verified: read :57-65]
scheme (:59) feeds only internal_url (:61) — the endpoint handed to the workers. The test's own client is pinned secure=False (:64). Setting UNSTRACT_MINIO_SCHEME=https therefore tells the workers to use TLS while leaving the test client plaintext: the two halves disagree, and the "TLS-fronted deployment can override" the comment promises isn't reachable.
This matters mainly because the var was introduced to clear the Sonar clear-text-HTTP finding — the literal moved, the plaintext client didn't. Either derive secure= from scheme too, or drop the var and suppress the Sonar rule on a compose-internal test fixture, which is the honest description of what's happening.
|
|
||
| ### Hermetic LLM (`UNSTRACT_LLM_MOCK_RESPONSE`) | ||
|
|
||
| Execute-path e2e tests must not call a real provider, so `ComposeRuntime.up()` sets `UNSTRACT_LLM_MOCK_RESPONSE` (default `MOCK_LLM_OK`) before boot. The test overlay forwards it into the workers, and `unstract.sdk1.llm` passes it to litellm as `mock_response`: litellm returns the string verbatim with fixed usage (10 prompt / 20 completion / 30 total), so both the answer and the token counts are exact-assertable. Sentinels like `litellm.RateLimitError` force error paths. Unset (the production default) the hook is a no-op. |
There was a problem hiding this comment.
P3 — two things this section states unconditionally hold only for the non-streaming path. [verified: test_connection read at llm.py:288-303; streaming usage measured against the pinned litellm 1.90.3]
1. 10 / 20 / 30 is completion-only. With stream=True, stream_options={"include_usage": True} — which is exactly what stream_complete passes (llm.py:554-558, and it is hooked at :546) — litellm returns 8 prompt / 3 completion / 11 total, not 10/20/30. The content assembles correctly; only the usage differs. Since the README presents 10/20/30 as a property of the mock and test_mock_response.py:77 calls it "litellm's defaults, asserted verbatim by the e2e tests", the next person to write a streaming assertion will trust it and get a confusing failure. No streaming test exists to pin it (the file's 7 tests all use the non-streaming path).
2. While the hatch is active, LLM adapter test-connection cannot pass. test_connection() calls self.complete() (llm.py:291), which now injects the mock, then regex-matches the response for "chennai" (:294). Under the rig's default MOCK_LLM_OK this never matches, so it raises the misleading "The credentials was valid however a sane response was not obtained" (:299-303).
Nothing today breaks — but adapter-register-llm is a tracked critical path currently listed as a gap, and this makes it unclosable in the e2e tier while the mock is set platform-wide. The PR description mentions test_connection; the README, which is the durable doc, doesn't. Worth adding beside the existing (good) note about embeddings being unhooked.
|
|
||
| @pytest.fixture(scope="session") | ||
| def api_deployment(provisioned_workflow: ProvisionedWorkflow) -> ApiDeployment: | ||
| """Deploy the provisioned workflow as an API, once for the whole group.""" |
There was a problem hiding this comment.
P3 — "once for the whole group" isn't what happens; provisioning runs once per xdist worker. [verified: tests/rig/groups.py:49 defaults parallel: bool = True, no e2e group in groups.yaml overrides it, and _pytest_command appends -n auto; the PR's own CI run shows these 3 tests on gw0/gw1/gw2]
Under -n auto each worker gets its own pytest session, so session-scoped fixtures run per worker — meaning provisioned_workflow and api_deployment execute up to 3 times for this group's 3 tests. Each cycle creates 4 adapters, a Prompt Studio project, an export, a workflow and a deployment against the shared platform.
It passes today (20.8s in CI), so this is not urgent. But the docstring is misleading, and 3x provisioning against one stack while each test holds a 300s poll is a plausible contributor to future timeout flakes under load. Either fix the docstring, or pin these groups to parallel: false given they already share a session-scoped platform.
jaseemjaskp
left a comment
jaseemjaskp
left a comment
There was a problem hiding this comment.
PR Review — hermetic LLM mock hook + execute-path e2e coverage
Ran the full PR Review Toolkit (code review, silent-failure hunt, type design, test coverage, comment analysis, simplification) plus independent verification of each claim against the code.
Overall this is unusually disciplined test infrastructure. The proof: marker ratchet is the strongest thing here: because passed_critical_path_ids counts only passing marked tests, a skipped or blocked test degrades to a reported gap rather than a silent green — that safety net catches most of what would otherwise be serious findings. The SDK hook itself is well-built: env-gated, never clobbers an explicit mock_response, applied uniformly at all four completion sites so there's no half-mocked path, value never logged. The e2e tests assert real system outputs (the answer arriving through the API, landing in the object store, written back to the Prompt Studio row) rather than asserting on the mock. No tautological tests found.
Three blocking findings:
test_api_deployment_fan_out.pynames its own false-pass mode in the docstring and then doesn't guard against it — andcritical_paths.yamlretires a declared gap on that test's word._blocked_resultnever reaches the CI report.cmd_reportrebuilds from junit only, and the blocked branch writes none — so the new ⏭️ status andblocked_bynote are invisible in the sticky PR comment, which is the only place they'd be read.e2e-api-deploymentcan be SIGKILLed before any test reports why: 3 × 300s poll budget against a 630s subprocess cap.
The rest are low/nit — mostly skip-paths that read green outside CI, a few comments whose stated reason is wrong even where the conclusion is right, and some duplication worth collapsing now that there are four e2e packages.
Findings verified against the code; already-posted review threads were cross-checked and excluded.
| } | ||
|
|
||
|
|
||
| @pytest.mark.critical_path("workflow-execution-fan-out") |
There was a problem hiding this comment.
[blocking] The fan-out test cannot fail for the one thing it exists to prove — and it retires a declared gap on that basis.
The module docstring names the false-pass mode outright (lines 3-5): "at the default of 1 every file lands in a single batch and this would pass while proving nothing." Nothing in the test then checks that premise. All four assertions hold identically for a fully serialised single-batch run:
by_name/sorted(by_name) == sorted(_DOCUMENTS)— 3 results either way_assert_totals_rejoined(:57) —total_files/successful_files/failed_filesare the same serially_assert_recorded_per_file(:71) —len(rows) == 3regardless of batching
Failure scenario: the overlay uses MAX_PARALLEL_FILE_BATCHES=${MAX_PARALLEL_FILE_BATCHES:-3}, so any of — a shell/CI export of 1 (and backend/sample.env:218 ships MAX_PARALLEL_FILE_BATCHES=1), a dropped overlay (tests/rig/runtime.py applies it only if COMPOSE_OVERLAY.exists()), or an org-level Configuration row (backend/configuration/enums.py:56, which overrides per-org) — silently reduces this to a slower duplicate of test_api_deployment_run. It still reports green.
This matters more than the usual "weak assertion" because tests/critical_paths.yaml flips workflow-execution-fan-out from covered_by: [] # gap to covered_by: [e2e-api-deployment] on the strength of this test. A degraded overlay silently retires a declared gap.
Suggested fix: assert the precondition rather than assuming it. Either read the effective value back (backend/configuration/internal_views.py serves exactly this key to workers) and fail if < 2, or assert observable parallelism in _assert_recorded_per_file — distinct batch/task identifiers on the per-file rows, or overlapping [created_at, modified_at] windows.
| # ── execution helpers ───────────────────────────────────────────────────────── | ||
|
|
||
|
|
||
| def _blocked_result(group: GroupDefinition, blocked_by: tuple[str, ...]) -> GroupResult: |
There was a problem hiding this comment.
[blocking] The new blocked status never reaches the report CI actually publishes.
Distinct from the exit-semantics thread below — this is about persistence, not gating.
_blocked_result is appended to the in-memory group_results only. The blocked branch continues before _execute_group, so no junit.xml and no exit.txt are written for the group. cmd_report (cli.py:316-321) rebuilds its group_results exclusively from parse_junit(name, tier, reports_dir) and drops anything returning None. The CI report job re-runs rig report over the downloaded artifacts (.github/workflows/ci-test.yaml:284-296).
Failure scenario: e2e-smoke goes red; e2e-etl / e2e-api-deployment / e2e-workflow / e2e-prompt-studio are correctly skipped and print [rig] SKIP … on the runner's stdout — but the sticky PR comment shows those groups simply missing, with no ⏭️ row and no "blocked by e2e-smoke" note. The blocked_by annotation added at reporting.py:251-263 and the ⏭️ icon are unreachable in CI; only the local run's stdout ever renders them. The feature is invisible exactly where it was meant to be read.
Suggested fix: in the blocked branch write a synthetic junit — _write_synthetic_junit already exists at cli.py:775 — with a skipped testcase plus exit.txt, or persist blocked_by into summary.json and have cmd_report merge it back over the junit-derived results.
| depends_on: [e2e-smoke] | ||
| optional: true | ||
|
|
||
| e2e-api-deployment: |
There was a problem hiding this comment.
[blocking] e2e-api-deployment's poll budget exceeds its group timeout, so a slow runner gets SIGKILLed before any test can report why.
The group inherits the 600s default (tests/rig/groups.py:48). tests/rig/cli.py:967 hard-caps the subprocess at timeout + 30 = 630s. But _POLL_TIMEOUT_SECONDS = 300 (tests/e2e/api_deployment/conftest.py:19) applies per test, and this group now has three of them (test_api_deployment_run, test_api_deployment_async, test_api_deployment_fan_out) — a worst case of ~900s of polling plus session provisioning.
Failure scenario: a loaded runner where the callback worker lags. _spawn kills the group at 630s and returns 124 before any test reaches its own pytest.fail(f"result never delivered within…"). No junit is written, so the group shows as a bare red row with no per-test detail, and all four critical paths it now claims (api-deployment-run, usage-token-tracking, callback-result-delivery, workflow-execution-fan-out) report as gaps/regressions with no clue which execution stalled.
Suggested fix: give e2e-api-deployment an explicit timeout_seconds above the summed poll budget (e.g. 1200, matching e2e-smoke), or drop _POLL_TIMEOUT_SECONDS to ~150s so 3 × poll + overhead fits inside 600s.
| def up(self) -> PlatformEndpoints: | ||
| if shutil.which("docker") is None: | ||
| raise RuntimeError("ComposeRuntime requires the `docker` CLI on PATH") | ||
| # setdefault so a CI/dev override wins. |
There was a problem hiding this comment.
[low] Only ComposeRuntime sets the mock var, and setdefault treats an exported empty string as "already set" — both routes end in the whole execute-path suite skipping green.
Two independent holes in the same two lines:
-
Non-compose runtimes never set it.
LocalRuntime.up()(:119) andTestcontainersRuntime.up()don't touchLLM_MOCK_RESPONSE_ENV, and_default_runtime_name()(:257) returnstestcontainerswheneverCIis unset. So for a developer running the e2e tier locally, or anyone on--runtime localagainst a dev stack,llm_mock_response(tests/e2e/conftest.py:91-96) skips and all six new execute-path tests skip. Every e2e group exits 0 → ✅ pass, and the five new critical paths quietly become gaps. CI is protected by--fail-on-critical-gap(ci-test.yaml:216); a localpython -m tests.rig run --tier e2eis not, and reads fully green with zero coverage. -
setdefaultaccepts"".UNSTRACT_LLM_MOCK_RESPONSE=counts as already-overridden. The empty string flows into the workers viadocker-compose.test.yaml:27/31(${UNSTRACT_LLM_MOCK_RESPONSE:-}), where_inject_mock_responsereads it as falsy and skips injection — andllm_mock_responsereads it as falsy and skips every test. One staleexport UNSTRACT_LLM_MOCK_RESPONSE=in a shell profile silently deletes the entire execute-path suite while the run stays green.
Suggested fix: if not os.environ.get(LLM_MOCK_RESPONSE_ENV): os.environ[LLM_MOCK_RESPONSE_ENV] = DEFAULT_LLM_MOCK_RESPONSE, hoisted into cmd_run for any runtime when platform groups are selected. And make the fixture fail rather than skip when the rig is driving — cmd_run already stamps UNSTRACT_RIG_SESSION_ID (cli.py:752) for precisely this distinction.
| if result is not None: | ||
| group_results.append(result) | ||
| # Tracked regardless of `optional` — see the cascade note above. | ||
| if exit_code not in _NON_FAILING_PYTEST_EXIT_CODES or ( |
There was a problem hiding this comment.
[low] A dependency that collected zero tests does not block its dependents.
_NON_FAILING_PYTEST_EXIT_CODES = (0, 5) (cli.py:52), so pytest's exit 5 — "no tests collected" — never lands in failed_groups. The gate here is e2e-smoke, whose entire job is answering "is the platform actually up?". A path typo in groups.yaml, a --marker filter that matches nothing, or a renamed test file turns that question into unanswered, and the new cascade treats it as yes — every dependent then runs against a platform nothing verified.
This is the same reasoning the sibling comment at cli.py:690-694 already applies to coverage attestation ("empty" … must not attest); the gating path just didn't inherit it. Either way the choice deserves to be explicit — the comment block above reasons only about red deps, not empty ones.
Suggested fix: treat exit 5 as failed for the purposes of failed_groups when the group is something else depends_on, and pin whichever semantics you pick with a cmd_run test (stub _execute_group to return (None, 5) for smoke, assert dependents do/don't run).
| @@ -66,6 +68,7 @@ paths: | |||
| - id: prompt-studio-fetch-response | |||
| description: "Prompt Studio: create project, add prompt, run single-pass, get response." | |||
There was a problem hiding this comment.
[nit] This path now attests a variant its covering test explicitly excludes.
The description says "run single-pass, get response", and the path was just flipped to proof: marker against test_prompt_studio_fetch_response.py. That test posts to fetch_response/ (not the single-pass route) and reads outputs with "is_single_pass_extract": "false" (tests/e2e/prompt_studio/test_prompt_studio_fetch_response.py:92). The registry is now recording coverage for something the test deliberately opts out of.
Suggest "Prompt Studio: create project, add prompt, run a prompt, get response.", or keep single-pass as a separate, still-uncovered path.
Same class of overstatement, milder, at :44 — workflow-create-execute ends "poll, fetch result", but test_workflow_execute.py's own docstring says it "asserts execution status rather than the answer, which a manual execute never exposes over HTTP".
| │ ├── api_deployment/ # API deployment e2e: sync, async, fan-out | ||
| │ ├── etl/ # ETL pipeline e2e (MinIO source + destination) | ||
| │ ├── prompt_studio/ # Prompt Studio fetch-response e2e | ||
| │ └── hurl/ # (future) hurl-based HTTP suites |
There was a problem hiding this comment.
[nit] "sync" overstates what's covered — all three api_deployment tests dispatch async.
Every module in tests/e2e/api_deployment/ calls dispatch_async(...) with timeout=0. test_api_deployment_run.py's own docstring says so explicitly: "Dispatches async and polls the status endpoint … which the sync POST cannot reliably wait out." The conftest comment at :15-18 says the same.
Suggest: # API deployment e2e: run, callback delivery, fan-out (all async).
| from types import ModuleType | ||
|
|
||
| # Stub python-magic so importing LLM does not depend on libmagic. | ||
| sys.modules.setdefault("magic", ModuleType("magic")) |
There was a problem hiding this comment.
[nit] The magic stub is inserted into sys.modules and never removed.
sys.modules.setdefault("magic", ModuleType("magic")) runs inside an lru_cached loader, so it persists for the rest of the process. If magic is installed but not yet imported when this module happens to run first, the empty stub shadows the real library for every subsequent sdk1 test — an ordering-dependent failure in an unrelated test, which is a nasty one to chase.
Suggest a fixture that pops the key it inserted (and only if it inserted it), or restrict the stub to the import itself via unittest.mock.patch.dict(sys.modules, ...).
| """Where the test and the workers each reach the object store.""" | ||
|
|
||
| client: object | ||
| bucket: str |
There was a problem hiding this comment.
[nit] client: object erases the only interface this fixture exists to provide.
Every consumer immediately calls methods object doesn't have — store.client.put_object(...), .list_objects(...), .get_object(...) at tests/e2e/etl/test_pipeline_etl_execute.py:51,98,103. No autocomplete, no arity checking, and mypy would reject all three call sites if it were turned on for tests/. The dataclass is the natural place to publish "this is a minio.Minio".
Two lines, and it keeps the runtime importorskip:
from typing import TYPE_CHECKING
if TYPE_CHECKING:
from minio import Minio
@dataclass(frozen=True)
class MinioFixture:
client: "Minio"Also in this block: endpoint is passed to minio.Minio raw, so someone following the neighbouring _INTERNAL_URL convention and exporting UNSTRACT_MINIO_ENDPOINT=http://localhost:9000 gets a ValueError at fixture setup rather than a skip — the one env var here that must not carry a scheme sits directly beside one that must.
| group = manifest.get(name) | ||
| blocked_by = tuple(sorted(manifest.transitive_deps(name) & failed_groups)) | ||
| if blocked_by: | ||
| print(f"\n[rig] SKIP {name} (dependency failed: {', '.join(blocked_by)})") |
There was a problem hiding this comment.
[nit] "dependency failed" isn't always true of the group it names.
The skip branch adds blocked groups to failed_groups, so a third-level group's blocked_by can name a dependency that was itself only blocked, never run. Given A red → B blocked → C blocked, the console prints [rig] SKIP C (dependency failed: B) — but B didn't fail, it was skipped. Same wording in _blocked_result's docstring and reporting.py:47.
Suggest "dependency failed or was blocked" in all three.
Separately, the comment above (:448-450) credits the weaker of the two gating mechanisms: a blocked group's path reports as a gap only when it wasn't green on the baseline — if it was, critical_paths.py:191 classifies it as a regression, and regressions fail unconditionally (cli.py:579-586) whereas gaps need --fail-on-critical-gap (cli.py:602). Worth saying "…reports as a gap (or a regression if it was green on the baseline) and gates accordingly."
One more, on duplication this PR surfaced: the CSRF-header helper now exists in seven near-copies across tests/e2e/ — and they've already drifted (tests/e2e/conftest.py:122 merges caller headers; tests/e2e/etl/conftest.py:81 silently discards them). A requests.Session subclass that stamps the header in request() would delete all seven call-site helpers.
muhammad-ali-e
left a comment
muhammad-ali-e
left a comment
There was a problem hiding this comment.
LGTM
Did a manual review since already done couple of AI reviews
|
|
||
| pytestmark = [pytest.mark.e2e, pytest.mark.critical] | ||
|
|
||
| _TERMINAL = {"COMPLETED", "ERROR", "STOPPED"} |
There was a problem hiding this comment.
NIT: can we set the TERMINAL statuses in a common place like /unstract/unstract.
| @property | ||
| def status(self) -> ResultStatus: | ||
| if self.blocked_by: | ||
| return "blocked" |
There was a problem hiding this comment.
NIT: would be better making these terms "blocked", "pass" etc are a constant.
| @property | ||
| def status(self) -> ResultStatus: | ||
| if self.blocked_by: | ||
| return "blocked" |
There was a problem hiding this comment.
NIT: would be better making these terms "blocked", "pass" etc are a constant.
`report` needs `changes` but never consulted its result. A failed filter job (paths-filter error, checkout failure, runner death) skips `test`, which the gate reads as a legitimate path-filter skip and reports success with no unit or integration tests having run. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017UVfw7aAocC3KCJaEyZ5GU
Rig: - Blocked groups now persist a synthetic junit carrying `rig-blocked-by`, so the cascade survives `rig report`, which rebuilds results from junit alone. Without it a blocked group parsed as a zero-count pass and falsely attested critical-path coverage. - GroupResult rejects a blocked row with non-zero counters. - Report statuses are module constants instead of scattered literals. - An exit-5 (no tests collected) group that others depend on now counts as failed, so dependents are blocked rather than run against nothing. - The LLM mock default moves to cmd_run so it applies to every runtime. - groups.py docstring no longer claims runtime gating is unimplemented. E2E: - Shared `wait_for_execution` helper replaces three near-identical polls; a CsrfSession subclass stamps X-CSRFToken so per-call helpers can't drift. - api_deployment poll: sleeps on every iteration, captures exceptions as evidence, treats 406 as a lost-200 acknowledge rather than a poll assert, and surfaces non-422 statuses as themselves. Timeout drops to 150s so the group's three tests fit one 600s budget. - MinIO scheme drives both the workers' URL and the client's TLS flag. - Missing rig-provisioned prerequisites fail instead of silently skipping. Core/SDK: - `ExecutionStatus.terminal_statuses()` names the terminal set. - Mock-response tests cover the empty-string no-op and assert every completion path still calls the injection hook. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017UVfw7aAocC3KCJaEyZ5GU
The e2e image build is the only CI check that runs `bun install` and `bun run build`; ci-frontend-lint only runs biome. Excluding frontend/** from e2e_relevant let a broken build or a desynced bun.lock merge green and surface on main via production-build.yaml. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017UVfw7aAocC3KCJaEyZ5GU
|
Unstract test resultsPer-group results
Critical paths✅ Covered critical paths
|
5 participants
Stacked on #2164. Adds the deterministic-LLM capability the execute-path critical-path gaps need, without dragging real provider secrets into CI.
What
llm.py): one module-level_inject_mock_response+ a one-line call at all fourlitellm.completion/acompletionsites. WhenUNSTRACT_LLM_MOCK_RESPONSEis set, litellm returns that string as the completion with fixed usage (10/20/30), tunable viaDEFAULT_MOCK_RESPONSE_*; error sentinels likelitellm.RateLimitErrorforce error paths. Unset in production → no-op.worker-executor-v2/worker-file-processing-v2(the processes that run the LLM). Empty unless the rig/CI sets it.Why this shape
test_connection(), so a fixed mock runs every execute path to completion.Not in this PR
The actual execute-path e2e tests (workflow-create-execute, usage-token-tracking, …) that consume this hook — those need the full compose stack booted to author the API flow correctly and will land next. Critical-path mappings are intentionally left unflipped until a real passing test exists (no false greens).
🤖 Generated with Claude Code