UN-3636 [MISC] Merge overlay manifests via UNSTRACT_RIG_EXTRA_MANIFESTS

[chandrasekharan-zipstack]

What

Three rig changes, bundled because they all land on the same test-infrastructure surface:

1. Overlay manifests. load_groups() merges extra group manifests listed in the UNSTRACT_RIG_EXTRA_MANIFESTS env var (os.pathsep-separated, REPO_ROOT-relative) onto the base tests/groups.yaml before validation — so cross-manifest depends_on and the platform-gate invariant are checked over the union. Group-name collisions are an error, not a silent override.
2. Cross-tier deps no longer re-run. --tier X now bounds depends_on expansion to tier X.
3. The ENVIRONMENT gate on the LLM mock is gone (merged in via UN-3636 [MISC] Drop the ENVIRONMENT gate on the LLM mock #2191), along with the now-unread ENVIRONMENT variable itself.

Why

1. Overlay manifests

Lets a downstream repo contribute its own test groups without editing the OSS manifest. First consumer is the cloud build: it copies a tests/groups.cloud.yaml into the merged tree (via copy_cloud_deps.py) and points the env var at it, so cloud-only plugin test groups (workers, etc.) join the rig without forking groups.yaml. No cloud specifics leak into OSS.

2. Cross-tier deps

integration-workflow-execution and e2e-smoke both declare depends_on: [unit-sdk1, unit-workers]. Because --tier expanded deps transitively with no tier bound, those two unit groups ran three times per CI run — once in the unit leg, again in integration, again in e2e. Measured on run 29891886572 (main):

group	unit leg	integration leg	e2e leg
unit-sdk1	21s	27s	27s
unit-workers	37s	51s	51s

≈136s of duplicate execution per run. On the cloud CI runner (custom-k8s-runner, 2 xdist workers) the same duplication costs ~350s.

3. Dropping the ENVIRONMENT gate (#2191)

Follow-up to #2170, removing a guard that PR added. The gate required ENVIRONMENT ∈ {test, development} on top of UNSTRACT_LLM_MOCK_RESPONSE before the SDK would mock a completion. It was added for this review comment — the hazard being a worker env block copied out of the test overlay into a real deployment, where litellm's synthetic 10/20/30 token usage would flow into the usage tables indistinguishable from real spend.

It did not defend that case:

• The gate was written into that same overlay block, next to the mock var, so a copy carries the gate along with it.
• docker/docker-compose.yaml set ENVIRONMENT=development on both workers that run the injection, so any deployment derived from base compose — which is how Unstract is self-hosted — satisfied the gate anyway.

That left one case it genuinely caught: the mock var set alone somewhere that sets no ENVIRONMENT at all. That held by accident (nothing sets the variable in k8s) rather than by design, and it cost a dependency on a variable nothing else in the codebase reads — so the variable was dropped from compose too, rather than left looking load-bearing.

What still guards the hatch is unchanged: it is off unless someone sets UNSTRACT_LLM_MOCK_RESPONSE, and it logs a warning once per process while active. The defence worth having is making mocked spend distinguishable downstream — an is_mock marker on the usage record, which config checks can't be copy-pasted around. Not in this PR.

Changes

Overlays

• tests/rig/groups.py: _extra_manifest_paths(), _load_manifest_dict(), _merge_manifest(), wired into load_groups().
• tests/rig/tests/test_groups.py: +6 tests (merge with cross-manifest depends_on, name collision, explicit-manifest isolation, overlay defaults, malformed overlay, missing overlay path).

Cross-tier deps

• tests/rig/selection.py: with --tier, drop dep-expanded groups outside that tier.
• tests/rig/tests/test_selection.py: +3 tests (cross-tier dep not pulled in, explicit request survives the filter, expansion unchanged without a tier filter).

LLM mock gate (#2191)

• unstract/sdk1/src/unstract/sdk1/llm.py: drop _ENVIRONMENT_ENV, _MOCK_ALLOWED_ENVIRONMENTS, _warn_mock_refused, and the gate.
• unstract/sdk1/tests/test_mock_response.py: drop the 4 gate tests and the autouse env fixture (9 tests remain, all passing).
• tests/compose/docker-compose.test.yaml: drop the two ENVIRONMENT=test lines added for the gate.
• docker/docker-compose.yaml: drop the unread ENVIRONMENT declarations.
• tests/README.md: record why it was removed, so it doesn't get re-added.

No-org short-circuit

• backend/utils/user_context.py: get_organization() returns None when no org is in context (import time, management commands with no request) instead of querying, so evaluating it on a DB-less/unmigrated setup can't fail — which is what the rig's backend groups do at collection time.
• backend/utils/tests/test_user_context.py: cover both branches, including the truthy one so inverting the guard can't pass unnoticed.

Safety

• Overlays are no-op when the env var is unset → zero effect on existing OSS runs, and they apply only to the default manifest, so an ad-hoc load_groups(path) stays isolated.
• Intra-tier deps still expand and topo-order (e2e-smoke → e2e-login); explicitly named groups are never dropped by the tier filter.
• Gating is unaffected: blocked_by intersects with groups that failed in the same run, so a dep that wasn't run simply doesn't gate — its own tier leg gates it.
• The LLM mock stays opt-in behind UNSTRACT_LLM_MOCK_RESPONSE and warns once per process while active.
• tests.rig validate OK — 18 groups, 16 critical paths. Full rig suite: 80 passed, 1 skipped.

🤖 Generated with Claude Code

https://claude.ai/code/session_01RnLaN45ShBbcCXWZkqCThz

[coderabbitai]

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Manifest loading now supports environment-specified overlays, tiered selection limits implicit dependency expansion, organization lookup skips database access without an identifier, and LLM mocking no longer depends on ENVIRONMENT.

Changes

Rig manifest and selection

Layer / File(s)	Summary
Default manifest overlay loading tests/rig/groups.py, tests/rig/tests/test_groups.py	Loads default-manifest overlays, merges defaults and groups, rejects collisions, validates inputs, and covers these behaviors with tests.
Tier-bounded dependency resolution tests/rig/selection.py, tests/rig/tests/test_selection.py	Filters implicit dependencies to the selected tier while preserving explicit selections and cross-tier expansion without a tier filter.

Organization lookup guard

Layer / File(s)	Summary
Organization lookup short-circuit backend/utils/user_context.py, backend/utils/tests/test_user_context.py	Returns None without database access for missing identifiers and retains database lookup for present identifiers.

LLM mock activation

Layer / File(s)	Summary
Environment-independent mock injection unstract/sdk1/src/unstract/sdk1/llm.py, unstract/sdk1/tests/test_mock_response.py, tests/README.md, tests/compose/docker-compose.test.yaml	Activates mock responses based only on UNSTRACT_LLM_MOCK_RESPONSE, retains the active warning, and updates related tests, documentation, and test services.
Compose environment cleanup docker/docker-compose.yaml	Removes explicit ENVIRONMENT=development entries from application and worker services.

Estimated code review effort: 3 (Moderate) | ~30 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CIEnvironment
  participant resolve
  participant Manifest
  CIEnvironment->>resolve: provide selected groups and tier
  resolve->>Manifest: expand selected dependencies
  Manifest-->>resolve: return expanded groups
  resolve->>resolve: filter implicit groups by tier
  resolve-->>CIEnvironment: return runnable groups

Loading

Suggested reviewers: muhammad-ali-e, ritwik-g

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title is concise and correctly highlights the main overlay-manifest change.
Description check	✅ Passed	It covers What/Why/Changes/Safety and testing, though some template headings are folded into prose or omitted.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/rig-extra-manifests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Greptile Summary

This PR updates the test rig overlay flow and related test-infrastructure behavior. The main changes are:

• Adds UNSTRACT_RIG_EXTRA_MANIFESTS support for merging overlay group manifests.
• Keeps explicit load_groups(path) calls isolated from overlays.
• Limits tier-specific dependency expansion to the selected tier.
• Removes the ENVIRONMENT gate from SDK LLM mock injection.
• Avoids organization DB lookup when no organization is in context.
• Adds tests for overlay merging, tier filtering, mock behavior, and user context lookup.

Confidence Score: 5/5

This looks safe to merge.

• No blocking issues found in the changed code.
• The explicit-manifest overlay isolation fix now keys off whether path was omitted, so explicit default-path loads stay isolated.

Important Files Changed

Filename	Overview
tests/rig/groups.py	Adds overlay manifest loading while keeping explicit manifest loads isolated.
tests/rig/tests/test_groups.py	Adds tests for overlay merge behavior, collisions, defaults, bad paths, and explicit-path isolation.
tests/rig/selection.py	Filters dependency-expanded groups outside the selected tier while preserving explicit requests.
unstract/sdk1/src/unstract/sdk1/llm.py	Removes the ENVIRONMENT check from mock-response injection.
backend/utils/user_context.py	Returns None without querying the database when no organization id is present.

_{Reviews (10): Last reviewed commit: "UN-3636 [FIX] Stop ide_prompt_complete t..." | Re-trigger Greptile}

[jaseemjaskp]

Automated PR review (PR Review Toolkit: code-reviewer, silent-failure-hunter, type-design-analyzer, pr-test-analyzer, comment-analyzer, code-simplifier).

Clean, well-tested change. The user_context short-circuit is behavior-preserving (all 42 callers already treat None as "no org") and adds no new silent failures; the overlay-manifest merge fails loud on every error path and rejects name collisions rather than silently overriding. No new types introduced. Approving — all findings below are non-blocking nits.

One pre-existing item worth a follow-up (not from this diff): get_organization catches a broad django.db.utils.ProgrammingError and returns None with no logging, which also swallows genuine runtime schema/permission errors under runserver — consider narrowing or logging.

Note: I did not repost @greptile-apps' P1 at groups.py:139 (overlay loop applies to any custom load_groups(path) caller) — it stands and is worth resolving; whatever fix lands there will need the two new rig tests updated, since they rely on overlays applying with an explicit path.

[greptile-apps]

Thanks for the thorough review. Taking the P1 at groups.py:139 — the fix is straightforward: guard the overlay loop on path is None. Overlays are only meaningful relative to the default manifest; a caller passing a custom path gets a self-contained load.

    # Overlay manifests are merged before validation, so cross-manifest
    # `depends_on` and the platform-gate invariant are checked over the union.
    # Overlays only apply when using the default manifest; a caller that passes
    # an explicit path gets a self-contained, isolated load.
    if path is None:
        for extra in _extra_manifest_paths():
            _merge_manifest(groups, extra, defaults)

The two new rig tests will need updating since they currently call load_groups(base) with an explicit path while also expecting overlays to fire. The simplest fix is to patch DEFAULT_MANIFEST instead:

def test_extra_manifest_merged(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
    base = _base_manifest(tmp_path)
    overlay = tmp_path / "groups.cloud.yaml"
    overlay.write_text(
        """
        version: 1
        groups:
          unit-cloud:
            tier: unit
            paths: [y]
            depends_on: [unit-base]
            optional: true
        """
    )
    monkeypatch.setattr("tests.rig.groups.DEFAULT_MANIFEST", base)
    monkeypatch.setenv("UNSTRACT_RIG_EXTRA_MANIFESTS", str(overlay))
    manifest = load_groups()  # path=None → uses patched DEFAULT_MANIFEST
    assert "unit-cloud" in manifest.names()
    assert "unit-base" in manifest.expand(["unit-cloud"])


def test_extra_manifest_name_collision_rejected(
    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
) -> None:
    base = _base_manifest(tmp_path)
    overlay = tmp_path / "groups.cloud.yaml"
    overlay.write_text(
        """
        version: 1
        groups:
          unit-base:
            tier: unit
            paths: [y]
            optional: true
        """
    )
    monkeypatch.setattr("tests.rig.groups.DEFAULT_MANIFEST", base)
    monkeypatch.setenv("UNSTRACT_RIG_EXTRA_MANIFESTS", str(overlay))
    with pytest.raises(ValueError, match="already defined"):
        load_groups()

This also makes the test semantics cleaner — it now exercises the real entry point (load_groups() with no args) rather than the explicit-path variant, which matches the actual production call site.

---

On the pre-existing get_organization broad ProgrammingError catch: agreed it's worth a follow-up. A DoesNotExist catch (or at minimum a logger.warning before the silent return None) would surface genuine schema/permission failures without breaking the no-org short-circuit added here.

[coderabbitai]

🧹 Nitpick comments (2)

tests/rig/tests/test_groups.py (2)

306-325: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Test explicit-manifest isolation with an invalid overlay path.

The valid overlay only proves that unit-cloud is not merged. Set UNSTRACT_RIG_EXTRA_MANIFESTS to a nonexistent path and assert load_groups(base) still succeeds; otherwise a regression could resolve overlays before honoring explicit-manifest isolation.

🤖 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/tests/test_groups.py` around lines 306 - 325, Update
test_explicit_manifest_ignores_overlays to set UNSTRACT_RIG_EXTRA_MANIFESTS to a
nonexistent overlay path instead of creating a valid overlay file. Assert
load_groups(base) succeeds and still excludes unit-cloud, verifying
explicit-manifest isolation occurs before overlay resolution.

---

251-265: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Cover multiple and REPO_ROOT-relative overlays.

This helper always registers one absolute path, so os.pathsep splitting and relative-path resolution remain untested. Add a case with two overlay entries, including one relative to REPO_ROOT, and assert both groups are merged.

🤖 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/tests/test_groups.py` around lines 251 - 265, Add a test covering
multiple overlays in the group-loading tests: create two overlay manifests,
register their paths in UNSTRACT_RIG_EXTRA_MANIFESTS joined with os.pathsep,
make one path relative to REPO_ROOT, and assert groups from both overlays are
merged. Reuse _overlay_env or adjust the test setup as needed while preserving
the existing default-manifest behavior.

🤖 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 `@tests/rig/tests/test_groups.py`:
- Around line 306-325: Update test_explicit_manifest_ignores_overlays to set
UNSTRACT_RIG_EXTRA_MANIFESTS to a nonexistent overlay path instead of creating a
valid overlay file. Assert load_groups(base) succeeds and still excludes
unit-cloud, verifying explicit-manifest isolation occurs before overlay
resolution.
- Around line 251-265: Add a test covering multiple overlays in the
group-loading tests: create two overlay manifests, register their paths in
UNSTRACT_RIG_EXTRA_MANIFESTS joined with os.pathsep, make one path relative to
REPO_ROOT, and assert groups from both overlays are merged. Reuse _overlay_env
or adjust the test setup as needed while preserving the existing
default-manifest behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4f05d44e-36ff-4f21-bd36-a023b3de2ec0

📥 Commits

Reviewing files that changed from the base of the PR and between ade772e and d48d1dc.

📒 Files selected for processing (4)

• backend/utils/tests/test_user_context.py
• backend/utils/user_context.py
• tests/rig/groups.py
• tests/rig/tests/test_groups.py

🚧 Files skipped from review as they are similar to previous changes (2)

• backend/utils/tests/test_user_context.py
• tests/rig/groups.py

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[github-actions]

Unstract test results

Per-group results

Status	Group	Tier	Passed	Failed	Errors	Skipped	Duration (s)
✅	e2e-api-deployment	e2e	3	0	0	0	20.6
✅	e2e-coowners	e2e	1	0	0	0	1.4
✅	e2e-etl	e2e	1	0	0	0	8.7
✅	e2e-login	e2e	2	0	0	0	1.1
✅	e2e-prompt-studio	e2e	1	0	0	0	4.7
✅	e2e-smoke	e2e	2	0	0	0	2.6
✅	e2e-workflow	e2e	1	0	0	0	16.3
✅	integration-backend	integration	124	0	0	27	60.6
✅	integration-connectors	integration	1	0	0	7	11.7
✅	unit-backend	unit	163	0	0	0	22.9
✅	unit-connectors	unit	63	0	0	0	9.9
✅	unit-core	unit	27	0	0	0	1.3
✅	unit-platform-service	unit	15	0	0	0	2.7
✅	unit-rig	unit	86	0	0	0	4.4
✅	unit-sdk1	unit	480	0	0	0	23.6
✅	unit-workers	unit	723	0	0	0	46.1
	TOTAL		1693	0	0	34	238.6

Critical paths

⚠️ Critical paths not yet covered

• workflow-execution-fan-out — Multi-file workflow execution fans out to file-processing workers and rejoins. (declared coverage: no groups declared)

✅ Covered critical paths

• auth-login — covered by e2e-login
• adapter-register-llm — covered by integration-backend
• workflow-author — covered by integration-backend
• co-owner-manage — covered by integration-backend, e2e-coowners
• workflow-create-execute — covered by e2e-workflow
• api-deployment-provision — covered by integration-backend
• api-deployment-auth — covered by integration-backend
• api-deployment-run — covered by e2e-api-deployment
• prompt-studio-author — covered by integration-backend
• prompt-studio-fetch-response — covered by e2e-prompt-studio
• connector-register-test — covered by integration-backend
• pipeline-etl-execute — covered by e2e-etl
• usage-aggregate-read — covered by integration-backend
• usage-token-tracking — covered by e2e-api-deployment
• callback-result-delivery — covered by e2e-api-deployment