refactor(e2e): seed-once model + run the stack via the root compose (migrate → seed → assert)

[ktursunov]

Rewrites the bronze→API E2E rig from a per-fixture loop (truncate → seed → dbt → reapply ~40 gold views → refresh → query, ×~50 fixtures) to a seed-once model: materialise every fixture's data once per session, then run each test as a pure assertion. Faster, simpler, and it exercises the real migration + compose path. Rebased on main (reconciles the analytics-api→analytics rename from #1579). 2 commits.

Commit 1 — seed-once rig (4a4234b9)

• Seed-once execution. Seed every fixture's namespaced bronze once → dbt build (staging → enrich → silver) once → reapply gold-view migrations once → refresh MVs once → each test just namespaces its request, POSTs /v1/metrics/queries, and checks expects. No per-test seed/dbt/migrate.
• Isolation by namespacing (lib/namespace.py): a per-fixture token rewrites the email domain and unique_key/id/source_id/department, applied identically to the seeded bronze and the request $filter. meta/test_seed_isolation.py proves no two fixtures share an RMT ORDER BY key — offline, before the stack is built.
• Run via the root compose, 3 phases: (1) up health-gated ClickHouse + MariaDB; (2) e2e-migrate runs the real src/ingestion/scripts/apply-ch-migrations.sh (the same entrypoint the Helm post-install hook uses); (3) e2e-runner seeds + asserts. Runs as an isolated insight-e2e compose project (own network/volumes/ports) so a developer's ./dev-compose.sh stack is untouched. analytics is spawned in-process by pytest after the world is built, so the runner needs no docker socket.
• Single version SSOT — reuses the root docker-compose.yml DB services, pinned to the versions the gitops charts deploy: ClickHouse 25.7.5, MariaDB 12.2.2. No second DB stack.
• Cleanup (large net deletion): removed compose/docker-compose.{yml,runner,cache}.yml, lib/compose.py, the dead migration_applier bootstrap, a dead+broken spawn() (called an undefined build()), and the write-only per-test TouchedLedger; renamed metrics/test_fixtures.py → test_metrics.py and the pytest marker fixture → metric (both clashed with pytest's own vocabulary).
• Three bugs fixed en route:
  1. analytics spawn PIPE deadlock — unread subprocess.PIPE stdout filled the 64 KB buffer during startup logging → process blocked → never served /health. Now sinks to a temp file.
  2. RMT id collapse — bronze_bamboohr.employees / bronze_jira.jira_issue are ORDER BY id with literal ids shared across fixtures → merge-collapse. Namespaced id; the guard now derives the real ORDER BY key from the placeholder DDL.
  3. Team bullets un-skipped — the two team-bullet fixtures filtered org_unit_id eq, which the analytics API never implemented (only org_unit_id in), so the predicate was silently dropped and the value blended company-wide. Scoping by person_id IN (roster) — how the API actually scopes team aggregates — isolates them. The skip list is removed entirely.

Commit 2 — request-scoping guard (b7ac15d4)

meta/test_request_scoping.py — a DB-free companion to test_seed_isolation.py that hardens the failure class the un-skip exposed, at author time:

• A. supported-filter — every fixture $filter clause must be a (field, operator) the API implements (person_id eq/in, org_unit_id in, metric_date ge/lt/le, drill_id/metric_key/section_id eq). Anything else is silently dropped by the API → unscoped result; this catches the exact org_unit_id eq trap.
• B. benchmark-scoping — a case asserting cohort stats (median/p25/p75/range/peer_n) must scope by identity (person_id/org_unit_id), so the cohort resolves to this fixture and not the whole shared world.

Validation

• CI green: 185 passed, 0 skipped (85 metric + framework tests, plus 100 guard items = 2 checks × 50 fixtures). CI builds the runner image and runs the full suite on GitHub runners.
• The ClickHouse 24.8 → 25.7 bump exposed one incompatibility (system.view_refreshes.last_refresh_result dropped) — fixed by switching refresh_intermediates to SYSTEM REFRESH VIEW + SYSTEM WAIT VIEW (race-free, CH 24.10+).

Notes for reviewers

• Compose structure diverges from refactor(analytics)!: rename analytics-api service to analytics, drop env-key crutches #1579. That PR kept the split compose/docker-compose.runner.yml + docker-compose.cache.yml; this PR consolidates them into one docker-compose.e2e.yml overlay layered on the root compose. Deliberate (one overlay, single version SSOT, less duplication) — flagging for whoever owns the e2e compose setup.
• MariaDB 11.4 → 12.2 is a major bump for the dev stack too (the SSOT trade-off). Existing dev mariadb-data volumes may need a reset; the e2e project uses ephemeral, project-scoped volumes so it's unaffected.
• Surfaced a live product bug (not fixed here): the analytics service silently drops unrecognised $filter clauses like org_unit_id eq, returning company-wide-but-200 results — filed as backend: analytics silently drops org_unit_id eq filter → whole-company aggregates (HTTP 200) #1657. This PR's guard prevents it in tests; the API-side fix is tracked separately.

Follow-ups (deferred)

• A second logical isolation plane (per-fixture metric_date window) + a small physical annex for any genuinely company-wide-cohort metric the current namespacing can't reach — only when a metric needs it (the guard flags such cases).
• Confirm whether the post-dbt reapply_migrations step is now redundant given fix(seed): single source of truth for ClickHouse placeholder DDL #1597 (placeholder-DDL SSOT).

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ffabda68-b1ec-4bb2-aeb5-b76d1b3a1837

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

This PR reworks the e2e test suite to a session-scoped "seed once, assert many" model with per-fixture namespacing for isolation, adds a metric coverage gate (script, CI job, e2e.sh command), enhances expect_engine to enforce full stat-field assertions, captures analytics-api logs, and updates all metric test YAML fixtures to a department-of-5 scenario.

Estimated code review effort: 4 (Complex) | ~75 minutes

Changes

Seed-once E2E framework and metric coverage gate

Layer / File(s)	Summary
Fixture identity namespacing src/ingestion/tests/e2e/lib/namespace.py, src/ingestion/tests/e2e/meta/test_seed_isolation.py	New module rewrites bronze records and request filters per fixture token; new test validates token uniqueness, email rewriting, and ORDER BY key disjointness.
Seed-once world builder & conftest orchestration src/ingestion/tests/e2e/lib/seed_once.py, src/ingestion/tests/e2e/conftest.py, src/ingestion/tests/e2e/api/test_session_smoke.py	Adds a single-pass world builder merging namespaced bronze and building staging/enrich/silver/gold once per session; conftest fixtures (ch_bootstrap, build_world, analytics_api) rewired accordingly.
Expect engine enforcement & assertion-only runner src/ingestion/tests/e2e/lib/expect_engine.py, src/ingestion/tests/e2e/meta/test_expect_engine.py, src/ingestion/tests/e2e/metrics/test_fixtures.py	Enforces that all present stat fields are asserted, adds tolerant numeric equality, and converts the metric smoke test into an assertion-only runner using namespaced requests.
Analytics API log capture src/ingestion/tests/e2e/lib/analytics_api.py	Redirects process stdout to a temp file and surfaces captured logs in startup/timeout errors.
Metric coverage gate & CI wiring src/ingestion/tests/e2e/lib/collect_metrics.py, src/ingestion/tests/e2e/lib/metric_coverage.py, src/ingestion/tests/e2e/e2e.sh, .github/workflows/e2e-bronze-to-api.yml, src/ingestion/tests/e2e/README.md, src/ingestion/tests/e2e/.gitignore	Adds catalog-snapshotting script, coverage report generator with skip-list hygiene checks, a gates e2e.sh command, and a new metric-coverage-gate CI job consuming an uploaded artifact.
Metric test YAML fixtures & templates src/ingestion/tests/e2e/metrics/*.test.yaml, src/ingestion/tests/e2e/metrics/templates/*	Rewrites AI, collaboration, task-delivery, and wiki metric fixtures to a five-member Engineering department scenario with erin custom-scope and empty-window cases; adds new team-bullet test files and dave/erin template entries.

Sequence Diagram(s)

sequenceDiagram
  participant Conftest as conftest.py
  participant SeedOnce as seed_once.build_world
  participant ClickHouse
  participant AnalyticsApi
  participant Tests as metric test cases

  Conftest->>SeedOnce: build_world(all_fixtures)
  SeedOnce->>SeedOnce: merge_namespaced_bronze
  SeedOnce->>ClickHouse: seed bronze, build staging/silver/gold once
  Conftest->>AnalyticsApi: start(build_world)
  Tests->>AnalyticsApi: namespaced request per fixture
  AnalyticsApi-->>Tests: response asserted via expect_engine
  Conftest->>AnalyticsApi: teardown -> collect_metrics -> stop

Loading

sequenceDiagram
  participant E2EJob as e2e job
  participant Artifact as coverage-inputs artifact
  participant GateJob as metric-coverage-gate job
  participant Gate as metric_coverage.py

  E2EJob->>Artifact: upload catalog_metrics.json
  GateJob->>Artifact: download coverage-inputs
  GateJob->>Gate: run against catalog_metrics.json
  Gate-->>GateJob: markdown report + exit code
  GateJob->>GateJob: append to step summary, fail if non-zero

Loading

Possibly related issues

• Metric smoke: Collaboration metrics #1441: Touches the same collaboration-metrics e2e test suite and shared fixtures/expectations addressed by this PR's metric-smoke rework.

Possibly related PRs

• constructorfabric/insight#1287: Both modify .github/workflows/e2e-bronze-to-api.yml's e2e job invocation/behavior.
• constructorfabric/insight#1507: Both touch conftest.py and expect_engine.py core framework code.
• constructorfabric/insight#1510: Both modify conftest.py session-start lifecycle/truncation behavior.

Suggested reviewers: cyberantonz

🚥 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 clearly captures the main change: moving E2E tests to a seed-once model and running the stack through compose.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.}

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (3)

src/ingestion/tests/e2e/lib/namespace.py (1)

72-81: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Namespacing is shallow — nested structures would silently bypass the rewrite (and the guard test).

namespace_record only inspects top-level rec.items() values; a bronze field whose value is a nested dict/list (rather than a flat string or a serialized JSON string) would not be scanned for @example.com or prefixed-field names. Since meta/test_seed_isolation.py's _iter_values uses the exact same shallow traversal, such a leak would not even be caught by the DB-free isolation guard — defeating its stated purpose of "asserting the invariant... fails fast... before the stack is ever built." Currently this is fine as long as every bronze fixture keeps identity data as strings/serialized JSON, but it's a silent failure mode for future fixtures.

♻️ Suggested defensive recursion

-def namespace_record(rec: dict, token: str) -> dict:
+def _rewrite_value(v, token: str):
+    if isinstance(v, str) and _EMAIL_DOMAIN in v:
+        return _domain_rewrite(v, token)
+    if isinstance(v, dict):
+        return {k: _rewrite_value(vv, token) for k, vv in v.items()}
+    if isinstance(v, list):
+        return [_rewrite_value(vv, token) for vv in v]
+    return v
+
+
+def namespace_record(rec: dict, token: str) -> dict:
     """Return a copy of a resolved bronze record with identity rewritten for `token`."""
     out: dict = {}
     for k, v in rec.items():
-        if isinstance(v, str) and _EMAIL_DOMAIN in v:
-            v = _domain_rewrite(v, token)  # emails anywhere, incl. JSON blobs
+        v = _rewrite_value(v, token)
         if k in _PREFIXED_FIELDS and isinstance(v, str) and v:
             v = f"{token}-{v}"
         out[k] = v
     return out

🤖 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 `@src/ingestion/tests/e2e/lib/namespace.py` around lines 72 - 81, The
namespacing logic in namespace_record is only shallow, so nested dict/list
values can bypass email and prefixed-field rewriting. Update namespace_record to
recurse through nested structures (and keep the same behavior for strings/JSON
blobs) so every embedded value is rewritten, and mirror that traversal in the
_iter_values guard used by meta/test_seed_isolation.py so the isolation check
catches nested leaks too.

src/ingestion/tests/e2e/meta/test_seed_isolation.py (1)

34-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

_PREFIXED duplicates namespace._PREFIXED_FIELDS — risk of silent drift.

This guard test hardcodes its own copy of the prefixed-field list instead of importing namespace._PREFIXED_FIELDS. If a future change adds/removes a field in one location but not the other, the guard could pass while namespacing is actually broken (or vice versa), undermining the test's purpose as the load-bearing isolation proof.

♻️ Suggested fix

-_PREFIXED = ("unique_key", "source_id", "department", "id")
+_PREFIXED = namespace._PREFIXED_FIELDS

Also applies to: 56-56

🤖 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 `@src/ingestion/tests/e2e/meta/test_seed_isolation.py` at line 34, The guard
test is duplicating the prefixed-field list instead of reusing the source of
truth, which can drift from namespace._PREFIXED_FIELDS. Update
test_seed_isolation to import and reference namespace._PREFIXED_FIELDS (or the
relevant namespace constant) in the _PREFIXED-based assertions and any related
checks, so the isolation test always validates the actual production list rather
than a hardcoded copy.

.github/workflows/e2e-bronze-to-api.yml (1)

113-142: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Consider persist-credentials: false on checkout steps that only read code.

Static analysis (zizmor artipacked) flags both this job's checkout (Line 126) and the e2e job's checkout (Line 51) for not setting persist-credentials: false. By default actions/checkout persists the GITHUB_TOKEN in local git config for the rest of the job; since neither job pushes commits, disabling persistence removes unnecessary residual credential exposure to any script/action running later in the same job. The concrete risk here is bounded (the uploaded artifact path is scoped to .artifacts/, not the whole workspace/.git), but it's a one-line, no-downside hardening.

🔒 Proposed fix

       - uses: actions/checkout@v4
+        with:
+          persist-credentials: false

Apply the same change to the checkout step in the e2e job (Line 51).

🤖 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/e2e-bronze-to-api.yml around lines 113 - 142, The checkout
steps in the `e2e` job and `metric-coverage-gate` job are persisting the
`GITHUB_TOKEN` unnecessarily, which leaves credentials available to later steps
in the same job. Update each `actions/checkout@v4` usage to disable credential
persistence by setting `persist-credentials: false`, keeping the change scoped
to the read-only checkouts in these jobs.

Source: Linters/SAST tools

🤖 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 `@src/ingestion/tests/e2e/conftest.py`:
- Around line 243-275: The _collect_metrics helper can still block or abort
teardown because it only checks subprocess return codes; update _collect_metrics
in conftest.py to be fully best-effort by adding a timeout to subprocess.run and
catching subprocess-related exceptions around the collect_metrics.py invocation.
Ensure any timeout or execution failure only logs a warning and returns so
analytics_api teardown can always continue to proc.stop() without being
interrupted.

In `@src/ingestion/tests/e2e/metrics/wiki_confluence_comments.test.yaml`:
- Around line 22-27: The wiki comment fixture is reusing the same page_id values
across fixtures, which can cause cross-matching in wiki_bullet_rows because it
joins on (tenant_id, page_id). Update the confluence wiki page entries in this
test fixture so each page_id is fixture-unique, either by prefixing the IDs here
or by applying the same namespacing in lib/namespace.py, and keep the related
unique_key values unchanged.

---

Nitpick comments:
In @.github/workflows/e2e-bronze-to-api.yml:
- Around line 113-142: The checkout steps in the `e2e` job and
`metric-coverage-gate` job are persisting the `GITHUB_TOKEN` unnecessarily,
which leaves credentials available to later steps in the same job. Update each
`actions/checkout@v4` usage to disable credential persistence by setting
`persist-credentials: false`, keeping the change scoped to the read-only
checkouts in these jobs.

In `@src/ingestion/tests/e2e/lib/namespace.py`:
- Around line 72-81: The namespacing logic in namespace_record is only shallow,
so nested dict/list values can bypass email and prefixed-field rewriting. Update
namespace_record to recurse through nested structures (and keep the same
behavior for strings/JSON blobs) so every embedded value is rewritten, and
mirror that traversal in the _iter_values guard used by
meta/test_seed_isolation.py so the isolation check catches nested leaks too.

In `@src/ingestion/tests/e2e/meta/test_seed_isolation.py`:
- Line 34: The guard test is duplicating the prefixed-field list instead of
reusing the source of truth, which can drift from namespace._PREFIXED_FIELDS.
Update test_seed_isolation to import and reference namespace._PREFIXED_FIELDS
(or the relevant namespace constant) in the _PREFIXED-based assertions and any
related checks, so the isolation test always validates the actual production
list rather than a hardcoded copy.

🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 87b78bd7-d0b7-4764-bd0b-26d5af8277bf

📥 Commits

Reviewing files that changed from the base of the PR and between c91ba98 and 1907ef1.

📒 Files selected for processing (59)

• .github/workflows/e2e-bronze-to-api.yml
• src/ingestion/tests/e2e/.gitignore
• src/ingestion/tests/e2e/README.md
• src/ingestion/tests/e2e/api/test_session_smoke.py
• src/ingestion/tests/e2e/conftest.py
• src/ingestion/tests/e2e/e2e.sh
• src/ingestion/tests/e2e/lib/analytics_api.py
• src/ingestion/tests/e2e/lib/collect_metrics.py
• src/ingestion/tests/e2e/lib/expect_engine.py
• src/ingestion/tests/e2e/lib/metric_coverage.py
• src/ingestion/tests/e2e/lib/namespace.py
• src/ingestion/tests/e2e/lib/seed_once.py
• src/ingestion/tests/e2e/meta/test_expect_engine.py
• src/ingestion/tests/e2e/meta/test_seed_isolation.py
• src/ingestion/tests/e2e/metrics/ai_ai_loc_share2.test.yaml
• src/ingestion/tests/e2e/metrics/ai_cc_active.test.yaml
• src/ingestion/tests/e2e/metrics/ai_cc_cost.test.yaml
• src/ingestion/tests/e2e/metrics/ai_cc_lines.test.yaml
• src/ingestion/tests/e2e/metrics/ai_cc_overage.test.yaml
• src/ingestion/tests/e2e/metrics/ai_cc_sessions.test.yaml
• src/ingestion/tests/e2e/metrics/ai_cc_tool_accept.test.yaml
• src/ingestion/tests/e2e/metrics/ai_cc_tool_acceptance.test.yaml
• src/ingestion/tests/e2e/metrics/ai_claude_web.test.yaml
• src/ingestion/tests/e2e/metrics/ai_cursor_acceptance.test.yaml
• src/ingestion/tests/e2e/metrics/ai_prs_total.test.yaml
• src/ingestion/tests/e2e/metrics/ai_prs_with_cc.test.yaml
• src/ingestion/tests/e2e/metrics/collab_active_days.test.yaml
• src/ingestion/tests/e2e/metrics/collab_emails_read.test.yaml
• src/ingestion/tests/e2e/metrics/collab_emails_received.test.yaml
• src/ingestion/tests/e2e/metrics/collab_emails_sent.test.yaml
• src/ingestion/tests/e2e/metrics/collab_files_engaged.test.yaml
• src/ingestion/tests/e2e/metrics/collab_files_shared_external.test.yaml
• src/ingestion/tests/e2e/metrics/collab_files_shared_internal.test.yaml
• src/ingestion/tests/e2e/metrics/collab_meeting_free.test.yaml
• src/ingestion/tests/e2e/metrics/collab_meeting_hours.test.yaml
• src/ingestion/tests/e2e/metrics/collab_meetings_count.test.yaml
• src/ingestion/tests/e2e/metrics/collab_teams_chats.test.yaml
• src/ingestion/tests/e2e/metrics/collab_teams_meeting_hours.test.yaml
• src/ingestion/tests/e2e/metrics/collab_teams_meetings.test.yaml
• src/ingestion/tests/e2e/metrics/collab_zulip_chat.test.yaml
• src/ingestion/tests/e2e/metrics/task_delivery_bugs_to_task_ratio_jira.test.yaml
• src/ingestion/tests/e2e/metrics/task_delivery_due_date_compliance_jira.test.yaml
• src/ingestion/tests/e2e/metrics/task_delivery_tasks_completed_jira.test.yaml
• src/ingestion/tests/e2e/metrics/team_bullet_collab_emails_sent.test.yaml
• src/ingestion/tests/e2e/metrics/team_bullet_task_delivery_tasks_completed.test.yaml
• src/ingestion/tests/e2e/metrics/templates/confluence_wiki_pages.yaml
• src/ingestion/tests/e2e/metrics/templates/jira_task.yaml
• src/ingestion/tests/e2e/metrics/templates/m365_email.yaml
• src/ingestion/tests/e2e/metrics/templates/outline_wiki_pages.yaml
• src/ingestion/tests/e2e/metrics/templates/people.yaml
• src/ingestion/tests/e2e/metrics/test_fixtures.py
• src/ingestion/tests/e2e/metrics/wiki_confluence_active_authors.test.yaml
• src/ingestion/tests/e2e/metrics/wiki_confluence_comments.test.yaml
• src/ingestion/tests/e2e/metrics/wiki_confluence_edits.test.yaml
• src/ingestion/tests/e2e/metrics/wiki_confluence_pages_created.test.yaml
• src/ingestion/tests/e2e/metrics/wiki_outline_active_authors.test.yaml
• src/ingestion/tests/e2e/metrics/wiki_outline_comments.test.yaml
• src/ingestion/tests/e2e/metrics/wiki_outline_edits.test.yaml
• src/ingestion/tests/e2e/metrics/wiki_outline_pages_created.test.yaml

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

_collect_metrics can block session teardown indefinitely; exceptions aren't caught.

The docstring states this is "Best-effort... never abort the session for it," but the implementation only handles a non-zero returncode:

• subprocess.run has no timeout=, so if collect_metrics.py hangs (network stall, deadlock, etc.), the call blocks forever. Since analytics_api's teardown is try: _collect_metrics(proc) finally: proc.stop(), the finally never gets a chance to run until the hang resolves — defeating the guarantee that the process always gets stopped.
• Any exception raised by subprocess.run itself (e.g. missing interpreter, permission error) propagates uncaught, again contradicting "never abort the session for it."

🛡️ Suggested fix

-    result = subprocess.run(
-        [
-            sys.executable,
-            str(script),
-            "--url",
-            proc.base_url,
-            "--out-dir",
-            str(out_dir),
-            "--tenant",
-            str(TEST_TENANT_ID),
-        ],
-        check=False,
-    )
-    if result.returncode != 0:
-        LOG.warning(
-            "coverage-artifact collection failed (rc=%d); gate jobs may lack inputs",
-            result.returncode,
-        )
+    try:
+        result = subprocess.run(
+            [
+                sys.executable,
+                str(script),
+                "--url",
+                proc.base_url,
+                "--out-dir",
+                str(out_dir),
+                "--tenant",
+                str(TEST_TENANT_ID),
+            ],
+            check=False,
+            timeout=60,
+        )
+        if result.returncode != 0:
+            LOG.warning(
+                "coverage-artifact collection failed (rc=%d); gate jobs may lack inputs",
+                result.returncode,
+            )
+    except Exception:
+        LOG.exception("coverage-artifact collection raised; gate jobs may lack inputs")

Also applies to: 301-307

🧰 Tools

🪛 ast-grep (0.44.0)

[error] 256-268: Command coming from incoming request
Context: subprocess.run(
[
sys.executable,
str(script),
"--url",
proc.base_url,
"--out-dir",
str(out_dir),
"--tenant",
str(TEST_TENANT_ID),
],
check=False,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(subprocess-from-request)

🪛 Ruff (0.15.20)

[error] 257-257: subprocess call: check for execution of untrusted input

(S603)

🤖 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 `@src/ingestion/tests/e2e/conftest.py` around lines 243 - 275, The
_collect_metrics helper can still block or abort teardown because it only checks
subprocess return codes; update _collect_metrics in conftest.py to be fully
best-effort by adding a timeout to subprocess.run and catching
subprocess-related exceptions around the collect_metrics.py invocation. Ensure
any timeout or execution failure only logs a warning and returns so
analytics_api teardown can always continue to proc.stop() without being
interrupted.