feat(ci): conditionally run CodeQL analysis workflows only when there are detected JS/Python file changes

[hainenber]

feat(ci): conditionally run CodeQL analysis workflows only when there are detected JS/Python file changes

SUMMARY

Add a step to dynamically generate the matrix used for running CodeQL analysis based on JS and/or Python file changes. Additionally, I removed the schedule trigger for this job since it got run quite often these AI days.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A

TESTING INSTRUCTIONS

Green CI

ADDITIONAL INFORMATION

• Has associated issue:
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • Migration is atomic, supports rollback & is backwards-compatible
  • Confirm DB migration upgrade and downgrade tested
  • Runtime estimates and downtime expectations provided
• Introduces new feature or API
• Removes existing feature or API

[bito-code-review]

Bito Automatic Review Skipped - Files Excluded

Bito didn't auto-review this change because all changed files are in the exclusion list for automatic reviews. No action is needed if you didn't intend for the agent to review it. Otherwise, to manually trigger a review, type /review in a comment and save.
You can change the excluded files settings here, or contact your Bito workspace admin at evan@preset.io.

[rusackas]

Thanks for tackling this, @hainenber! One thing before this can merge, though.

Looks like set-lang-matrix builds the lang_matrix string but never writes it to $GITHUB_OUTPUT, just echoes it to the log. So needs.changes.outputs.lang_matrix will come through empty, and fromJson() on that in analyze's matrix is going to blow up the first time a PR actually touches python or JS. This PR's own CI didn't catch it since it only touches workflow yaml, so analyze never ran to exercise that path. Could you fix that and test against a branch that actually changes python or JS files, so we know the matrix comes out right before this lands?

[sadpandajoe]

Even after this step is wired up to $GITHUB_OUTPUT, the matrix still won't build correctly:

1. changes.outputs above only declares python and frontend — there's no lang_matrix entry, so needs.changes.outputs.lang_matrix in the analyze job stays empty no matter what this step writes.
2. jq '[inputs] | tostring' serializes the array into a JSON string (e.g. "[\"python\",\"javascript\"]"), not a bare array. fromJson(...) on that value returns a string, not the list matrix.language needs.

Could you add lang_matrix: ${{ steps.set-lang-matrix.outputs.lang_matrix }} to this job's outputs: block and drop | tostring so the step emits a plain JSON array?

[hainenber]

Good catch! Fixed in 9783919

[sadpandajoe]

This narrows CodeQL coverage versus the previous hardcoded matrix: python/frontend are directory-based change-detector groups, not a language classifier. A change limited to a .js file living outside superset-frontend/ — e.g. superset/mcp_service/index.js, or a .js file under scripts/ — sets python=true and leaves frontend=false, so this matrix will only run the python CodeQL analysis and silently skip javascript, even though a JavaScript file changed. The old ["python", "javascript"] matrix always ran both. Is that coverage gap intentional, or should the matrix reflect the actual language of the changed files rather than reusing the changes/frontend groups?

[hainenber]

should the matrix reflect the actual language of the changed files rather than reusing the changes/frontend groups?

I agree with this, it's a coverage gap and IMO, should be patched in a follow-up PR.

[hainenber]

Created #44822 for tracking

[sadpandajoe]

Tracking this in a follow-up still leaves JavaScript changes outside superset-frontend/ without a JavaScript scan after this merges. Could we keep both languages in the matrix until the detector can classify those files correctly?

[hainenber]

IMO, I think JS changes outside of superset-frontend not getting CodeQL scan is at most a status quo and would be improved once said follow-up PR fix it and we won't have to keep in mind to revisit this part again. It's more linear in terms of action items :D

[sadpandajoe]

The base workflow runs both languages when a superset/ or scripts/ file changes, whereas this matrix runs only Python for a JavaScript file in either directory. Restoring the daily full scan helps after merge but still removes that JavaScript check from the PR; could this keep both languages until the language-aware detector is wired in?

[hainenber]

Since we have lang-classifying detector fix merged in, we can proceed now. I'll need to incorporate it though

[sadpandajoe]

The merged detector adds a separate languages output, but this head still builds the matrix from python/frontend, so rebasing alone would still skip JavaScript outside the frontend directory. Could you incorporate the detector change and wire its languages output into both matrix selection and the analysis gate?

[hainenber]

Incorporated in 770b157

[netlify]

✅ Deploy Preview for superset-docs-preview ready!

Name	Link
🔨 Latest commit	770b157
🔍 Latest deploy log	https://app.netlify.com/projects/superset-docs-preview/deploys/6ac1377de92ca40008b44b65
😎 Deploy Preview	https://deploy-preview-44699--superset-docs-preview.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.68%. Comparing base (549508b) to head (770b157).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #44699      +/-   ##
==========================================
- Coverage   82.28%   81.68%   -0.60%     
==========================================
  Files        2996     2979      -17     
  Lines      185109   182920    -2189     
  Branches    42834    41960     -874     
==========================================
- Hits       152316   149425    -2891     
- Misses      30031    30733     +702     
  Partials     2762     2762              

Flag	Coverage Δ
hive	36.30% <ø> (ø)
mysql	55.32% <ø> (ø)
postgres	55.33% <ø> (ø)
presto	38.17% <ø> (ø)
python	86.23% <ø> (ø)
sqlite	55.05% <ø> (ø)
unit	79.23% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[hainenber]

Could you fix that and test against a branch that actually changes python or JS files, so we know the matrix comes out right before this lands?

I made a placeholder change in a Python test file and you can see the CodeQL analysis ran only for Python lang :D

[bito-code-review]

Code Review Agent Run #9d831f

Actionable Suggestions - 0

Review Details

• Files reviewed - 1 · Commit Range: 9c88534..9783919

  • tests/common/assert_utils.py
• Files skipped - 1

  • .github/workflows/codeql-analysis.yml - Reason: Filter setting
• Tools

  • MyPy (Static Code Analysis) - ✔︎ Successful
  • Astral Ruff (Static Code Analysis) - ✔︎ Successful
  • Whispers (Secret Scanner) - ✔︎ Successful
  • Detect-secrets (Secret Scanner) - ✔︎ Successful

---

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

• /review - Manually triggers an incremental AI Review.

• /review full - Manually triggers a full AI Review.

• /pause - Pauses automatic reviews on this pull request.

• /resume - Resumes automatic reviews.

• /resolve - Marks all Bito-posted review comments as resolved.

• /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

• Customize agent settings
• Review rules
• General documentation
• FAQ

AI Code Review powered by

[sadpandajoe]

On a push containing multiple commits, the change detector only compares the final commit with its parent, so a Python change earlier in the push can be omitted when the tip only changes frontend code; this matrix then skips the Python scan, and the removed schedule no longer catches it later. Could this derive changed files from the push's full before-to-after range instead?

[hainenber]

Oh if that's the case then it's quite a glaring issue with existing change_detector.py logic. No doubt that should be fixed but IMO, it's a bit scope-creeping if we include the changes here. I've created #44822 for tracking

[sadpandajoe]

The detector bug predates this PR, but using its output to drop a language scan and removing the scheduled fallback makes it a coverage regression here. Could we retain the full language matrix or the scheduled full scan until the push-range fix lands?

[hainenber]

This I agree. Let's have back the harness for all-changes scheduled trigger.

[hainenber]

Resolved in 8bd6565