Skip to content

ci: multi-language coverage gate (Rust, .NET, Python) - #1503

Merged
ktursunov merged 5 commits into
constructorfabric:mainfrom
ktursunov:ci-coverage
Jun 29, 2026
Merged

ci: multi-language coverage gate (Rust, .NET, Python)#1503
ktursunov merged 5 commits into
constructorfabric:mainfrom
ktursunov:ci-coverage

Conversation

@ktursunov

@ktursunov ktursunov commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

What

Adds a local-script coverage gate spanning Rust, .NET, and the Python CDK connectors — modeled on constructorfabric/gears-rust, but without any external coverage service. The same script runs locally and in CI, so the gate behaves identically in both.

How it works

Cobertura XML is the common format — the one report type all three toolchains emit (cargo-llvm-cov, Coverlet / XPlat Code Coverage, pytest-cov) — so the gate engine is language-agnostic. The engine and its config are a single self-contained file, scripts/ci/coverage.py: the component↔path map and thresholds are module-level constants, no separate config file.

Two gates, both a flat 80%:

  1. Per-component overall — when a PR changes a component, that component's line coverage must be ≥ 80%.
  2. New code — changed lines must be ≥ 80% covered, via diff-cover against origin/main, reported per component.

Only components a PR actually changes are judged, so unrelated components never gate it. The new-code report is grouped by language (Rust / Python / .NET).

Components (13)

  • Rust (5): insight-clickhouse, oidc-authn-plugin, analytics-api, api-gateway, jira-enrich
  • .NET (1): identity
  • Python CDK connectors (7): gitlab, github, github-v2, bitbucket-cloud, hubspot, salesforce, github-copilot

Nocode/YAML connectors are excluded (no first-party code to cover).

CI topology

  • changes — detects which components a PR touches by reusing coverage.py's component↔path map (single source of truth; no globs duplicated in YAML) and emits a per-language matrix.
  • Producersrust (matrix, one job per crate via cargo llvm-cov --package), dotnet, and python (matrix, one job per connector). Each runs only for changed components and uploads a Cobertura artifact. GitHub has no native per-job path filter; this is the canonical detect-job + if:/fromJson pattern (the same shape as dorny/paths-filter), sourcing the path map from our SSOT.
  • rust-lintcargo fmt + clippy -D warnings, kept separate so the lint gate is one check rather than repeated across the matrix.
  • coverage-gate — downloads all Cobertura artifacts and runs both gates; judges only components that produced a report; writes a per-language markdown report to the job summary (no external service, no PR comment — fork PRs only get a read-only token). Runs even when some producers were skipped (!cancelled()).

All actions run on the Node 24 runtime.

Behavior on this PR

This PR only touches CI infrastructure (scripts/ci/**, .github/workflows/ci.yml, .gitignore), which maps to no component — so the changes job emits empty matrices, no producers run, and the gate passes with nothing to judge. Enforcement begins on the next PR that changes component code.

Follow-ups

  • Make coverage-gate (and rust-lint) required status checks in branch protection.

Summary by CodeRabbit

  • New Features

    • Added a more targeted CI coverage check that only runs for changed backend, connector, and Python areas.
    • Introduced a consolidated coverage report with pass/fail gating for overall and changed-code coverage.
  • Chores

    • Removed older, separate backend and connector test workflows.
    • Updated ignored files so generated coverage artifacts are not tracked.

@coderabbitai

coderabbitai Bot commented Jun 26, 2026

Copy link
Copy Markdown

Review Change Stack

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

Walkthrough

Replaces backend-checks.yml and connector-enrich-tests.yml with a new unified ci.yml that uses a diff-driven component registry (scripts/ci/components.py), a change-detection script (scripts/ci/changed.py), and a coverage gate script (scripts/ci/coverage.py) to run per-language coverage producers and enforce overall and new-code coverage thresholds.

Changes

Diff-driven CI coverage gate

Layer / File(s) Summary
Component registry and change detection
scripts/ci/components.py, scripts/ci/changed.py
components.py defines ROOT, COMPARE_BRANCH, COMPONENTS, and component_for. changed.py maps git diff output to per-language JSON matrices with Rust workspace fanout lint logic.
Cobertura parsing and coverage measurement
scripts/ci/coverage.py (lines 1–225)
Normalizes Cobertura class paths across multiple source roots, merges all *.xml reports, buckets line hits by component, renders PASS/FAIL table, and builds a markdown summary per language section.
Patch gate, cmd_gate, and CLI
scripts/ci/coverage.py (lines 230–394)
Runs diff-cover JSON output, buckets new-code lines per component, enforces NEW_CODE_MIN, implements cmd_gate with --require/--no-patch/--summary, and wires the gate subcommand CLI.
Unified ci.yml workflow
.github/workflows/ci.yml
New workflow with path-filtered PR triggers and workflow_dispatch, changes job running changed.py, Rust/dotnet/Python producer matrix jobs uploading Cobertura artifacts, and coverage-gate consumer downloading and evaluating those artifacts.
Deleted workflows, reference updates, and gitignore
.github/workflows/e2e-bronze-to-api.yml, src/backend/clippy.toml, .gitignore
backend-checks.yml and connector-enrich-tests.yml deleted; comment in e2e workflow and clippy.toml header updated to reference ci.yml; coverage output directories added to .gitignore.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

Possibly related PRs

  • constructorfabric/insight#1429: Directly related — the previous implementation of backend-checks.yml Rust/dotnet coverage reporting that this PR replaces with the new diff-driven flow.

Suggested reviewers

  • cyberantonz
  • mitasovr

🐇 A new gate stands tall in the CI lane,
Diff-driven and sharp, it checks every gain.
Old workflows deleted, the coverage reborn,
Components registered, no lost file to mourn.
With patch lines and tables and markdown so bright,
The rabbit hops proudly — the coverage is right! ✅

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 73.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: a CI coverage gate spanning Rust, .NET, and Python.
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.
✨ 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

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

@ktursunov ktursunov changed the title ci: multi-language coverage gate (Codecov-free) ci: multi-language coverage gate Jun 26, 2026
@ktursunov ktursunov changed the title ci: multi-language coverage gate ci: multi-language coverage gate (Rust, .NET, Python) Jun 26, 2026
@ktursunov
ktursunov marked this pull request as ready for review June 26, 2026 16:10
@ktursunov
ktursunov requested a review from a team as a code owner June 26, 2026 16:10

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)

171-174: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Pin the coverage CLI tool versions.

Line 173 and Line 244 resolve whatever dotnet-coverage and diff-cover release is current that day. That makes the required coverage gate drift independently of the repo and can disagree with local runs. Lock these tool versions in the workflow or a repo-managed tool manifest.

Also applies to: 243-244

🤖 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.yml around lines 171 - 174, The CI workflow is
installing coverage tools without fixed versions, so the coverage gate can
change over time and diverge from local runs. Update the workflow steps that
install dotnet-coverage and diff-cover to pin explicit versions, or move both
tools into a repo-managed tool manifest so the versions are controlled by the
repository. Keep the existing install steps recognizable by the “Install
dotnet-coverage” and “Install diff-cover” names so they are easy to locate.
🤖 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 @.github/workflows/ci.yml:
- Around line 14-18: The workflow fan-out in the backend path handling is
missing coverage for shared Rust files like src/backend/clippy.toml, so these
changes can bypass rust-lint and Rust coverage entirely. Update the
path-to-component mapping used by the jobs fan-out so shared backend paths are
either mapped to all affected Rust components or routed through a dedicated
fallback that triggers the necessary Rust checks. Make the fix in the backend
path selection and component emission logic so rust-lint and coverage always run
for workspace-level backend Rust files.
- Line 50: All third-party actions in this workflow are still referenced by
mutable tags, so update every uses entry to the exact commit SHA for the
intended release version. Review each action reference in the workflow and
replace the tag-based selectors with pinned SHAs, including the checkout and all
other action steps listed in the review, so the CI pipeline no longer depends on
retargetable upstream tags.
- Line 110: The checkout steps in the rust, dotnet, and python jobs currently
persist the repository token in the workspace, which is unsafe for jobs that
later upload PR-derived artifacts. Update each `actions/checkout@v5` usage in
`.github/workflows/ci.yml` to set `persist-credentials: false` so the token is
not written to `.git/config`. Keep the change scoped to the checkout steps used
by those artifact-producing jobs.

In @.gitignore:
- Line 69: The .gitignore comment references the wrong coverage script path;
update the note in the coverage-gate entry to point to scripts/ci/coverage.py
instead of scripts/coverage.py. Keep the ignore rules unchanged and only correct
the descriptive comment so it matches the workflow used by the coverage gate.

In `@scripts/ci/coverage.py`:
- Around line 513-536: In cmd_gate, do not treat components with total == 0 as
automatically skipped when they were actually changed; instead, derive the
changed component set and fail the gate for any changed component missing a
report or covered lines. Update the logic around measured_all/measured so
cmd_gate can identify missing components, print their names, and set comp_pass =
False when any are absent. Also make sure the patch gate path in run_patch_gate
is not bypassed just because no XML reports were loaded if there are changed
components to evaluate.
- Around line 144-147: The Cobertura parsing in parse_cobertura() is using the
unsafe standard XML parser, so switch the ElementTree import in
scripts/ci/coverage.py to defusedxml.ElementTree and keep the parse logic
unchanged. Then update the CI jobs that run this script, including coverage-gate
and any changes job that invokes it, to install defusedxml after setup-python
and before calling scripts/ci/coverage.py so the hardened parser is available at
runtime.

---

Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 171-174: The CI workflow is installing coverage tools without
fixed versions, so the coverage gate can change over time and diverge from local
runs. Update the workflow steps that install dotnet-coverage and diff-cover to
pin explicit versions, or move both tools into a repo-managed tool manifest so
the versions are controlled by the repository. Keep the existing install steps
recognizable by the “Install dotnet-coverage” and “Install diff-cover” names so
they are easy to locate.
🪄 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: a7b33bf5-e9cc-424b-b0ab-27ab363ca68b

📥 Commits

Reviewing files that changed from the base of the PR and between 636f64d and d8fa7c71cd6d14e7bd0652f0961b3293bb2012ac.

📒 Files selected for processing (6)
  • .github/workflows/backend-checks.yml
  • .github/workflows/ci.yml
  • .github/workflows/e2e-bronze-to-api.yml
  • .gitignore
  • scripts/ci/coverage.py
  • src/backend/clippy.toml
💤 Files with no reviewable changes (1)
  • .github/workflows/backend-checks.yml

Comment thread .github/workflows/ci.yml
Comment thread .github/workflows/ci.yml
dotnet: ${{ steps.detect.outputs.dotnet }}
python: ${{ steps.detect.outputs.python }}
steps:
- uses: actions/checkout@v5

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major

Pin every third-party action to a commit SHA.

All uses: references in this workflow rely on mutable tags (e.g., @v5, v6, @stable). This violates supply-chain security best practices by exposing the CI pipeline to upstream tag retargeting. Update every action reference to use the specific commit SHA for the intended version.

This applies to:
Line 50, Line 55, Line 77, Line 82, Line 87, Line 110, Line 115, Line 121, Line 130, Line 134, Line 140, Line 153, Line 155, Line 159, Line 164, Line 182, Line 198, Line 200, Line 204, Line 216, Line 232, Line 239, Line 247.

🧰 Tools
🪛 zizmor (1.26.1)

[warning] 50-52: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)


[error] 50-50: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

🤖 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.yml at line 50, All third-party actions in this
workflow are still referenced by mutable tags, so update every uses entry to the
exact commit SHA for the intended release version. Review each action reference
in the workflow and replace the tag-based selectors with pinned SHAs, including
the checkout and all other action steps listed in the review, so the CI pipeline
no longer depends on retargetable upstream tags.

Comment thread .github/workflows/ci.yml
Comment thread .gitignore Outdated
Comment thread scripts/ci/coverage.py
Comment thread scripts/ci/coverage.py
Comment on lines +513 to +536
def cmd_gate(args) -> int:
reports_dir = Path(args.reports_dir) if args.reports_dir else COVERAGE_DIR
files = load_all_reports(reports_dir, ROOT)
measured_all, unbucketed = measure(files, COMPONENTS)
# Only judge components that actually ran (produced a report → total > 0).
# Unchanged components are skipped by CI, so they have no report — don't fail
# them at 0/0; they were gated when they last changed.
measured = {n: v for n, v in measured_all.items() if v[1] > 0}
skipped = sorted(n for n, v in measured_all.items() if v[1] == 0)

print(f"\n=== Per-component overall coverage gate (>= {OVERALL_MIN}%) ===\n")
if measured:
comp_pass = print_table(measured)
else:
comp_pass = True
print("No component reports — nothing to gate.")
if skipped:
print(f"\nskipped (no diff / not run): {', '.join(skipped)}")
if unbucketed:
print(f"\nnote: {unbucketed} covered file(s) matched no component (ignored).")

patch_pass, patch_per_comp, patch_output = True, {}, ""
if not args.no_patch and files:
patch_pass, patch_per_comp, patch_output = run_patch_gate(reports_dir, COMPONENTS)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Gate changed components, not only components that emitted XML.

Line 520 drops every component with total == 0, and Line 535 skips the patch gate entirely when no reports were loaded. Combined with producers/artifact uploads that can succeed without XML, a PR changing an untested component can pass as “nothing to gate” instead of failing at 0% coverage. Recompute the changed component set in cmd_gate and fail if any changed component has no report/covered lines.

Possible direction
 def cmd_gate(args) -> int:
     reports_dir = Path(args.reports_dir) if args.reports_dir else COVERAGE_DIR
     files = load_all_reports(reports_dir, ROOT)
     measured_all, unbucketed = measure(files, COMPONENTS)
-    # Only judge components that actually ran (produced a report → total > 0).
-    # Unchanged components are skipped by CI, so they have no report — don't fail
-    # them at 0/0; they were gated when they last changed.
-    measured = {n: v for n, v in measured_all.items() if v[1] > 0}
+    changed = {
+        name
+        for names in changed_components(COMPARE_BRANCH, COMPONENTS).values()
+        for name in names
+    }
+    measured = {n: v for n, v in measured_all.items() if n in changed and v[1] > 0}
+    missing = sorted(n for n in changed if measured_all.get(n, (0, 0, 0.0))[1] == 0)
     skipped = sorted(n for n, v in measured_all.items() if v[1] == 0)

Then mark comp_pass = False when missing is non-empty and print the missing component names.

🤖 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 `@scripts/ci/coverage.py` around lines 513 - 536, In cmd_gate, do not treat
components with total == 0 as automatically skipped when they were actually
changed; instead, derive the changed component set and fail the gate for any
changed component missing a report or covered lines. Update the logic around
measured_all/measured so cmd_gate can identify missing components, print their
names, and set comp_pass = False when any are absent. Also make sure the patch
gate path in run_patch_gate is not bypassed just because no XML reports were
loaded if there are changed components to evaluate.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🤖 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/backend/libs/insight-clickhouse/src/lib.rs`:
- Around line 107-114: Remove the temporary exported demo API from
insight-clickhouse before merge: the pub demo_new_code_uncovered function should
not remain in the crate’s public surface. Delete or move this coverage-gate-only
logic into test-only/private code, and make sure any references to
demo_new_code_uncovered or its #[must_use] export are removed so the library
exposes only real APIs.

In `@src/backend/plugins/oidc-authn-plugin/src/lib.rs`:
- Around line 14-30: The temporary coverage demo helper is currently exposed
from the library API, which should not be part of the production crate surface.
Remove demo_new_code_covered from the public library surface in lib.rs, and if
validation is still needed, move the helper and its demo_gate_tests module under
#[cfg(test)] so they are compiled only for tests; otherwise delete both the
helper and the test entirely before merging.

In `@src/backend/services/identity/src/Insight.Identity.Domain/CoverageDemo.cs`:
- Around line 3-28: The new helper in CoverageDemo is intentionally uncovered
and is causing the identity coverage gate to fail. Either add a unit test that
exercises CoverageDemo.Fold with representative positive and non-positive
inputs, or remove the CoverageDemo class entirely before merging so the
uncovered lines are no longer part of the diff.

In `@src/ingestion/connectors/crm/hubspot/source_hubspot/_demo_gate.py`:
- Around line 1-13: The demo fixture in demo_new_code_uncovered is intentionally
keeping the coverage gate red because it introduces uncovered lines in the
hubspot connector. Remove the _demo_gate.py module entirely, or replace it with
real covered code and add tests for demo_new_code_uncovered so the new-code
coverage check no longer sees uncovered lines in this connector.

In `@src/ingestion/connectors/git/gitlab/source_gitlab/_demo_gate.py`:
- Around line 1-9: Remove the temporary demo module from the gitlab connector
package before merge. The helper in _demo_gate.py, including
demo_new_code_uncovered, is only for exercising the coverage gate and should not
remain in the shipped codebase. Delete the file and ensure no references to this
demo-only surface remain in the gitlab connector package.

In `@src/ingestion/connectors/git/gitlab/source_gitlab/config.py`:
- Around line 47-56: Remove the temporary test scaffolding helper from the
production config module by deleting the _coverage_gate_demo function and its
related temporary comment block in config.py. Make sure no runtime code in this
module depends on _coverage_gate_demo, and keep only the real GitLab source
configuration logic in place.
🪄 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: 0a3145da-47f6-469a-aa22-d1cba5b2129b

📥 Commits

Reviewing files that changed from the base of the PR and between d8fa7c71cd6d14e7bd0652f0961b3293bb2012ac and 6b12e8dfb0c5968071a369f77e984f016f1a80ba.

📒 Files selected for processing (13)
  • .github/workflows/ci.yml
  • .github/workflows/connector-enrich-tests.yml
  • .github/workflows/e2e-bronze-to-api.yml
  • scripts/ci/changed.py
  • scripts/ci/components.py
  • scripts/ci/coverage.py
  • src/backend/libs/insight-clickhouse/src/lib.rs
  • src/backend/plugins/oidc-authn-plugin/src/lib.rs
  • src/backend/services/identity/src/Insight.Identity.Domain/CoverageDemo.cs
  • src/ingestion/connectors/crm/hubspot/source_hubspot/_demo_gate.py
  • src/ingestion/connectors/git/gitlab/source_gitlab/_demo_gate.py
  • src/ingestion/connectors/git/gitlab/source_gitlab/config.py
  • src/ingestion/connectors/git/gitlab/tests/test_coverage_demo.py
💤 Files with no reviewable changes (1)
  • .github/workflows/connector-enrich-tests.yml
✅ Files skipped from review due to trivial changes (1)
  • .github/workflows/e2e-bronze-to-api.yml

Comment thread src/backend/libs/insight-clickhouse/src/lib.rs Outdated
Comment thread src/backend/plugins/oidc-authn-plugin/src/lib.rs Outdated
Comment thread src/backend/services/identity/src/Insight.Identity.Domain/CoverageDemo.cs Outdated
Comment thread src/ingestion/connectors/crm/hubspot/source_hubspot/_demo_gate.py Outdated
Comment thread src/ingestion/connectors/git/gitlab/source_gitlab/_demo_gate.py Outdated
Comment thread src/ingestion/connectors/git/gitlab/source_gitlab/config.py Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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 @.github/workflows/ci.yml:
- Around line 52-54: The checkout steps for the changes and coverage-gate jobs
still persist the GitHub token in .git/config; update those actions/checkout@v5
usages to disable credential persistence like the rust, dotnet, and python jobs.
Keep fetch-depth: 0 if needed, but add persist-credentials: false to the
relevant checkout blocks so the hardening is consistent across the workflow.

In `@scripts/ci/coverage.py`:
- Around line 267-279: The diff-cover invocation in the coverage flow should
fail fast on tool errors before parsing the JSON report. In the function that
runs subprocess.run for diff-cover and then reads json_path, check
proc.returncode immediately after the run and exit with a clear message if it is
non-zero, since --fail-under 0 does not cover input/tool failures. Keep the JSON
loading and bucket_patch logic only for successful diff-cover runs.
🪄 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: 1f194d27-b741-43d2-9bc3-09fabd7af9ce

📥 Commits

Reviewing files that changed from the base of the PR and between 6b12e8dfb0c5968071a369f77e984f016f1a80ba and 5953f11a0fb2d653fc28772cdec3b97f1c00fea7.

📒 Files selected for processing (16)
  • .github/workflows/backend-checks.yml
  • .github/workflows/ci.yml
  • .github/workflows/connector-enrich-tests.yml
  • .github/workflows/e2e-bronze-to-api.yml
  • .gitignore
  • scripts/ci/changed.py
  • scripts/ci/components.py
  • scripts/ci/coverage.py
  • src/backend/clippy.toml
  • src/backend/libs/insight-clickhouse/src/lib.rs
  • src/backend/plugins/oidc-authn-plugin/src/lib.rs
  • src/backend/services/identity/src/Insight.Identity.Domain/CoverageDemo.cs
  • src/ingestion/connectors/crm/hubspot/source_hubspot/_demo_gate.py
  • src/ingestion/connectors/git/gitlab/source_gitlab/_demo_gate.py
  • src/ingestion/connectors/git/gitlab/source_gitlab/config.py
  • src/ingestion/connectors/git/gitlab/tests/test_coverage_demo.py
💤 Files with no reviewable changes (2)
  • .github/workflows/backend-checks.yml
  • .github/workflows/connector-enrich-tests.yml
✅ Files skipped from review due to trivial changes (4)
  • .gitignore
  • src/ingestion/connectors/git/gitlab/source_gitlab/_demo_gate.py
  • .github/workflows/e2e-bronze-to-api.yml
  • src/backend/clippy.toml
🚧 Files skipped from review as they are similar to previous changes (7)
  • src/backend/services/identity/src/Insight.Identity.Domain/CoverageDemo.cs
  • src/ingestion/connectors/git/gitlab/source_gitlab/config.py
  • src/ingestion/connectors/git/gitlab/tests/test_coverage_demo.py
  • scripts/ci/components.py
  • src/backend/plugins/oidc-authn-plugin/src/lib.rs
  • src/ingestion/connectors/crm/hubspot/source_hubspot/_demo_gate.py
  • src/backend/libs/insight-clickhouse/src/lib.rs

Comment thread .github/workflows/ci.yml
Comment on lines +52 to +54
- uses: actions/checkout@v5
with:
fetch-depth: 0

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Disable credential persistence in the changes and coverage-gate checkouts too.

The rust, dotnet, and python jobs already set persist-credentials: false, but these two checkouts (both fetch-depth: 0) still leave the repo token in .git/config. zizmor flags both (artipacked). Apply the same hardening for consistency.

🛡️ Minimal hardening
       - uses: actions/checkout@v5
         with:
           fetch-depth: 0
+          persist-credentials: false

Also applies to: 302-304

🧰 Tools
🪛 zizmor (1.26.1)

[warning] 52-54: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)


[error] 52-52: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

🤖 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.yml around lines 52 - 54, The checkout steps for the
changes and coverage-gate jobs still persist the GitHub token in .git/config;
update those actions/checkout@v5 usages to disable credential persistence like
the rust, dotnet, and python jobs. Keep fetch-depth: 0 if needed, but add
persist-credentials: false to the relevant checkout blocks so the hardening is
consistent across the workflow.

Source: Linters/SAST tools

Comment thread scripts/ci/coverage.py
Adds a per-component coverage gate spanning Rust, .NET, and the Python CDK
connectors, modeled on gears-rust but without any external coverage service —
the same logic runs locally and in CI.

Tooling (scripts/ci/, single source of truth):
- components.py: the component<->path registry + component_for (longest-prefix).
- coverage.py: processes Cobertura reports and reports coverage per component;
  two gates, each 80% — per-component overall line coverage, and new-code
  (patch) coverage via diff-cover. Runs no tests. Cobertura XML is the
  language-agnostic intermediate format; parsed via defusedxml.
- changed.py: emits the per-language CI matrix (rich entries) for the changes
  job; --all emits every component for a manual full/baseline run.

Workflow (.github/workflows/ci.yml, replaces backend-checks.yml):
- diff-triggered: a changes job maps the PR diff to components and emits dynamic
  per-language matrices, so only changed components run; a shared src/backend
  Rust change (clippy.toml, Cargo.*) triggers rust-lint only, with no coverage
  fanout (per-component isolation preserved).
- producer jobs collect coverage inline (cargo llvm-cov / dotnet --collect +
  merge / pytest-cov) and upload one Cobertura artifact each; the coverage-gate
  job downloads them and runs the gate, writing a per-language markdown report
  to the job summary.
- coverage-gate fails if change detection or any changed producer failed, and
  --require fails if a changed component produced no report (no silent skips).
- workflow_dispatch full=true runs the whole matrix (baseline).
- actions on Node-24 majors; producer checkouts use persist-credentials false.

Removes connector-enrich-tests.yml (the only enrich crate, jira-enrich, is
fully covered by its coverage job).

Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
…ust-lint)

Unifies Rust CI into one job per crate that runs fmt + clippy (when entry.lint) and coverage (when entry.cover). changed.py tags each rust matrix entry: a service change lints+covers only itself; a shared lib/plugin or workspace-level file (clippy.toml, Cargo.*) fans lint out to every backend crate (cross-crate compile net) while coverage stays per-changed-crate; jira-enrich is now linted too (default features). -D warnings stays on clippy only. The gate's --require ignores lint-only entries. Lint is now enforced via coverage-gate (its guard fails on any rust matrix failure), so rust-lint need not be a separate required check.

Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 @.github/workflows/ci.yml:
- Around line 257-273: The pytest exit-5 path is currently treated as success
without generating a coverage XML, which makes the required-component gate in
scripts/ci/coverage.py fail as an incomplete report. Update the CI workflow
around the pytest invocation and Upload Cobertura step so that the
exit-5/no-tests case still produces a real 0%-coverage report for the component,
or otherwise remove the special-casing and let it fail consistently; use the
existing matrix.entry.name, matrix.entry.cov_package, and scripts/ci/coverage.py
requirement flow to locate the affected logic.
🪄 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: e619ef1f-8390-443e-9395-ef03061bbc8a

📥 Commits

Reviewing files that changed from the base of the PR and between 101da692582e273d7a7075a4af52124065759e81 and 5ad641c.

📒 Files selected for processing (9)
  • .github/workflows/backend-checks.yml
  • .github/workflows/ci.yml
  • .github/workflows/connector-enrich-tests.yml
  • .github/workflows/e2e-bronze-to-api.yml
  • .gitignore
  • scripts/ci/changed.py
  • scripts/ci/components.py
  • scripts/ci/coverage.py
  • src/backend/clippy.toml
💤 Files with no reviewable changes (2)
  • .github/workflows/connector-enrich-tests.yml
  • .github/workflows/backend-checks.yml
✅ Files skipped from review due to trivial changes (3)
  • src/backend/clippy.toml
  • .github/workflows/e2e-bronze-to-api.yml
  • .gitignore
🚧 Files skipped from review as they are similar to previous changes (1)
  • scripts/ci/components.py

Comment thread .github/workflows/ci.yml
…st lint

jira-enrich was never linted (the old rust-lint only covered src/backend);
folding fmt+clippy into the per-crate matrix surfaced pre-existing rustfmt
drift and clippy::doc_markdown lints. Auto-fixed with cargo fmt + clippy --fix
(generated by a CI toolchain; no local rust). Behavior-preserving.

Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>


jira-enrich's strict [lints.clippy] (pedantic/unwrap_used/expect_used/dbg_macro)
was never CI-enforced and the code violates it extensively (mostly test-code
unwraps). Mark the crate clippy=false in components.py so the unified Rust job
runs fmt + coverage but skips clippy, unblocking the gate. Tracked by constructorfabric#1512;
re-enable once the debt is cleared. Other crates keep full fmt+clippy.

Signed-off-by: Konstantin Tursunov <Konstantin.Tursunov@constructor.tech>
@ktursunov

Copy link
Copy Markdown
Contributor Author

Follow-up: jira-enrich clippy debt (#1512)

Folding fmt+clippy into the per-crate Rust matrix surfaced that jira-enrich is the one crate that isn't clippy-clean. It declares a strict policy in its Cargo.toml [lints.clippy] (pedantic = deny, unwrap_used/expect_used/dbg_macro = deny) that was never CI-enforced — the old rust-lint job only covered the src/backend workspace, and jira-enrich is a separate workspace.

In this PR:

  • jira-enrich's rustfmt drift is fixed (auto-applied).
  • jira-enrich's clippy is temporarily silenced (clippy: false on its entry in scripts/ci/components.py) — it still runs fmt + coverage; every other crate gets full fmt+clippy+coverage.

The clippy cleanup (fix violations / scope the policy for test code, then remove the clippy: false flag) is tracked in #1512, assigned to @mitasovr.

@ktursunov
ktursunov merged commit b6e6238 into constructorfabric:main Jun 29, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants