Modernize Makefile and CI workflow by marcpaterno · Pull Request #615 · LSSTDESC/firecrown

marcpaterno · 2026-02-08T17:53:54Z

Description

Refactor the Makefile to use native parallel execution and improve alignment with the CI system. This ensures consistency between local development and remote verification environments.

Added new targets including test-ci, docs-verify, and improved unit-tests with per-component 100% coverage checks and data merging. These provide a reliable way to run the full project suite before pushing.

Fixed race conditions in parallel documentation builds and added explicit dependencies for correctness. Included a detailed comment in the Makefile explaining the sequential tutorial build decision.

Updated CI configuration to call unified Makefile targets, reducing duplication and maintenance overhead. This includes standardizing the use of conda environments in GitHub Actions.

Introduced CONTRIBUTING.md with setup instructions, recommended developer workflows, and a visual target relationship diagram. Improved make help to better guide new developers toward common tasks.

Updated the install target to safely pre-uninstall existing versions and added aliases like typecheck for better discoverability.

Type of change

Refactoring: this PR changes no code at all

Checklist:

The following checklist will make sure that you are following the code style and
guidelines of the project as described in the
contributing page.

I have run bash pre-commit-check and fixed any issues
I have added tests that prove my fix is effective or that my feature works
I have made corresponding changes to the documentation
I have 100% test coverage for my changes (please check this after the CI system has verified the coverage)

Refactor the Makefile to use native parallel execution and improve alignment with the CI system. This ensures consistency between local development and remote verification environments. Added new targets including test-ci, docs-verify, and improved unit-tests with per-component 100% coverage checks and data merging. These provide a reliable way to run the full project suite before pushing. Fixed race conditions in parallel documentation builds and added explicit dependencies for correctness. Included a detailed comment in the Makefile explaining the sequential tutorial build decision. Updated CI configuration to call unified Makefile targets, reducing duplication and maintenance overhead. This includes standardizing the use of conda environments in GitHub Actions. Introduced CONTRIBUTING.md with setup instructions, recommended developer workflows, and a visual target relationship diagram. Improved make help to better guide new developers toward common tasks. Updated the install target to safely pre-uninstall existing versions and added aliases like typecheck for better discoverability.

codecov · 2026-02-08T18:11:10Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.0%. Comparing base (5d36572) to head (9bbb8b1).
⚠️ Report is 12 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #615   +/-   ##
=======================================
  Coverage   100.0%   100.0%           
=======================================
  Files         151      151           
  Lines        9044     9044           
  Branches     1033     1033           
=======================================
  Hits         9044     9044

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

Restructure CI into a three-stage pipeline (quick validation, full compatibility matrix, downstream/documentation) to provide faster feedback while preserving comprehensive cross-platform testing. Add Conda caching keyed by the environment file hash and date to make setup more reliable and faster, with conditional env update/save steps on cache misses. Ensure linters run against the installed editable package for accurate checks, add a macOS RPATH workaround, and make external dependency/docs jobs depend on the main matrix to avoid wasted runs.

Copilot

Pull request overview

This PR modernizes the project’s developer/CI entrypoints by centralizing lint/test/docs workflows in the Makefile, then updating GitHub Actions and contributor docs to use those unified targets.

Changes:

Enable Make-driven parallelism and refactor lint/unit-test/docs targets (including CI-aligned targets like test-ci and docs-verify).
Update GitHub Actions CI workflow to call the unified Makefile targets and adjust job dependencies.
Add CONTRIBUTING.md documenting setup, recommended workflows, and target relationships.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.

File	Description
`Makefile`	Introduces default parallelism via `MAKEFLAGS`, adds CI/docs verification targets, and restructures lint/unit test orchestration.
`CONTRIBUTING.md`	Adds contributor setup + workflow guidance and a Make target dependency diagram.
`.github/workflows/ci.yml`	Switches CI steps to `make` targets and restructures the workflow into staged jobs.

Comments suppressed due to low confidence (1)

.github/workflows/ci.yml:35

The push trigger is limited to the master branch. This repo appears to use main as the default branch (PR diffs are against main), so CI may not run on pushes/merges to the default branch. Update the push branch filter to the actual default branch (and keep CONTRIBUTING consistent).

name: firecrown-ci
on:
  push:
    branches:
      - "master"
    paths-ignore:

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-08T18:47:45Z

Makefile

 ##@ Pre-commit

-pre-commit: format lint test-coverage docs  ## Run all pre-commit checks (format, lint, test with coverage, docs)
+pre-commit: format lint docs-verify test-ci ## Run all pre-commit checks


With parallel make enabled by default, pre-commit: format lint docs-verify test-ci can run format concurrently with lint/tests/docs. Since format modifies files, this can cause non-deterministic lint/test results. Make this target sequential (e.g., use a recipe that invokes the sub-targets in order, or use GNU make .WAIT/.NOTPARALLEL appropriately).

Suggested change

pre-commit: format lint docs-verify test-ci ## Run all pre-commit checks

pre-commit: ## Run all pre-commit checks

$(MAKE) format

$(MAKE) lint

$(MAKE) docs-verify

$(MAKE) test-ci

Copilot · 2026-02-08T18:47:45Z

Makefile

+test-all-coverage: unit-tests-pre unit-tests-core unit-tests-post ## Run core tests with coverage (fast)
+
+unit-tests-core:  ## Internal target for core tests with coverage
+	$(PYTEST) -vv --cov firecrown --cov-report xml --cov-branch -n auto


test-all-coverage’s prerequisites can execute in parallel: unit-tests-core and unit-tests-post may run at the same time, and unit-tests-pre is not guaranteed to finish before either starts. This can lead to races on .coverage* files and flaky/incorrect coverage.xml. Add explicit ordering (e.g., make unit-tests-core depend on unit-tests-pre, and unit-tests-post depend on unit-tests-core, or run them sequentially in a single recipe).

Suggested change

test-all-coverage: unit-tests-pre unit-tests-core unit-tests-post ## Run core tests with coverage (fast)

unit-tests-core: ## Internal target for core tests with coverage

$(PYTEST) -vv --cov firecrown --cov-report xml --cov-branch -n auto

test-all-coverage: ## Run core tests with coverage (fast)

$(MAKE) unit-tests-pre

$(MAKE) unit-tests-core

$(MAKE) unit-tests-post

Copilot · 2026-02-08T18:47:46Z

.github/workflows/ci.yml

-            python -m pytest -vv --runslow -n auto
+            make test-all
          fi
      - name: Running example tests


In non-coverage matrix runs, this step executes make test-all, and test-all already includes test-example in the Makefile. The subsequent “Running example tests” step runs make test-example again, duplicating work/time. Either remove the separate example step or change the non-coverage branch to run a target that excludes examples.

Suggested change

- name: Running example tests

- name: Running example tests

if: ${{ matrix.coverage == true }}

Copilot · 2026-02-08T18:47:46Z

CONTRIBUTING.md

+| `make lint -j` | Run all linters (`black`, `flake8`, `mypy`, `pylint`) in parallel | Before every commit |
+| `make test -j` | Run fast unit tests in parallel | Regularly during development |
+| `make unit-tests -j` | Run all unit tests with 100% per-component coverage check | Before pushing |
+| `make test-ci` | Run the full test suite exactly as the CI system does | Final check before pushing |
+| `make docs -j` | Build and verify all documentation (tutorials + API) | When changing tutorials or docstrings |
+| `make pre-commit` | A comprehensive check: format, lint, docs-verify, and full tests | Recommended pre-push check |
+
+> [!TIP]
+> Always use the `-j` flag (e.g., `make -j lint`) to take full advantage of parallel execution. The `Makefile` automatically detects the number of available CPUs.


The workflow recommends make lint -j / make test -j and the TIP suggests always using -j without a job count. In GNU make, -j with no number means “unlimited jobs”, which can overload a developer machine/CI runner—especially since the Makefile already enables parallelism by default. Recommend using JOBS=N make <target> (or make -jN) instead of bare -j, and update the table accordingly.

Suggested change

| `make lint -j` | Run all linters (`black`, `flake8`, `mypy`, `pylint`) in parallel | Before every commit |

| `make test -j` | Run fast unit tests in parallel | Regularly during development |

| `make unit-tests -j` | Run all unit tests with 100% per-component coverage check | Before pushing |

| `make test-ci` | Run the full test suite exactly as the CI system does | Final check before pushing |

| `make docs -j` | Build and verify all documentation (tutorials + API) | When changing tutorials or docstrings |

| `make pre-commit` | A comprehensive check: format, lint, docs-verify, and full tests | Recommended pre-push check |

> [!TIP]

> Always use the `-j` flag (e.g., `make -j lint`) to take full advantage of parallel execution. The `Makefile` automatically detects the number of available CPUs.

| `JOBS=N make lint` | Run all linters (`black`, `flake8`, `mypy`, `pylint`) in parallel | Before every commit |

| `JOBS=N make test` | Run fast unit tests in parallel | Regularly during development |

| `JOBS=N make unit-tests` | Run all unit tests with 100% per-component coverage check | Before pushing |

| `make test-ci` | Run the full test suite exactly as the CI system does | Final check before pushing |

| `JOBS=N make docs` | Build and verify all documentation (tutorials + API) | When changing tutorials or docstrings |

| `make pre-commit` | A comprehensive check: format, lint, docs-verify, and full tests | Recommended pre-push check |

> [!TIP]

> The `Makefile` enables parallel execution by default, so `make <target>` will already use multiple CPUs where appropriate. If you want to control the level of parallelism, set an explicit job count, for example `JOBS=4 make lint` or `make -j4 lint`, rather than using bare `-j`.

Copilot · 2026-02-08T18:47:46Z

CONTRIBUTING.md

+1. **Create a Branch**: Always work on a new branch for your feature or bug fix.
+2. **Write Tests**: Ensure your changes are covered by unit tests. We aim for 100% coverage on new code.
+3. **Verify Locally**: Run `make pre-commit` to ensure everything is in order.
+4. **Submit PR**: Once your tests pass locally, submit a Pull Request to the `master` branch.


This refers to submitting PRs to the master branch, but the repository appears to use main as the default branch (PR diffs are against main). Update this to the correct default branch name to avoid confusing contributors.

Suggested change

4. **Submit PR**: Once your tests pass locally, submit a Pull Request to the `master` branch.

4. **Submit PR**: Once your tests pass locally, submit a Pull Request to the `main` branch.

Copilot · 2026-02-08T18:47:46Z

Makefile

+docs-verify: docs-generate-symbol-map docs-code-check docs-symbol-check docs-linkcheck ## Run all documentation verification checks

-docs: docs-generate-symbol-map docs-build docs-linkcheck docs-code-check docs-symbol-check ## Build and check all documentation (tutorials + API docs)
-	@echo ""
-	@echo "Documentation built and checked successfully:"
-	@echo "  - Tutorials: $(TUTORIAL_OUTPUT_DIR)/"
-	@echo "  - API docs: $(DOCS_BUILD_DIR)/html/index.html"
+docs-code-check: tutorials ## Check Python code blocks in .qmd files
+	@echo "Checking tutorial code blocks for syntax errors..."
+	@$(PYTHON) $(FIRECROWN_PKG_DIR)/fctools/code_block_checker.py $(TUTORIAL_DIR) || (echo "❌ docs-code-check failed" && exit 1)


docs-code-check (and docs-symbol-check) depend on tutorials, which forces a full Quarto render even though the Python checkers only parse the .qmd sources and the generated symbol_map.json. This makes it impossible to run these verification tools without Quarto installed and slows down fast feedback loops. Consider removing the tutorials prerequisite from these check targets (keep docs-generate-symbol-map where needed), and let docs-verify/docs-build be the targets that require a full render.

Copilot · 2026-02-08T18:47:47Z

Makefile

+JOBS ?= $(shell nproc 2>/dev/null || sysctl -n hw.ncpu 2>/dev/null || echo 4)
+MAKEFLAGS += -j$(JOBS)


MAKEFLAGS += -j$(JOBS) is applied unconditionally, which can override/negate a user’s explicit make -j1 ... (and the help text suggests -j1 works for serial debugging). Consider only adding -j$(JOBS) when no -j is already present in $(MAKEFLAGS), so command-line job control remains authoritative.

Suggested change

JOBS ?= $(shell nproc 2>/dev/null || sysctl -n hw.ncpu 2>/dev/null || echo 4)

MAKEFLAGS += -j$(JOBS)

JOBS ?= $(shell nproc 2>/dev/null || sysctl -n hw.ncpu 2>/dev/null || echo 4)

# Only add a default -j setting if the user has not already specified one

ifeq ($(filter -j%,$(MAKEFLAGS)),)

MAKEFLAGS += -j$(JOBS)

endif

Replace the auto-generated help target with a curated quick reference organized by workflow stage. The new help output guides developers through common tasks: - During development: format, lint, test - Before committing: unit-tests, docs - Before pushing: pre-commit, test-ci The original comprehensive target list is now available as help-all for users who need to see all targets.

Enhance Makefile with coverage ID support for concurrent test runs, implement safe parallel execution with clean target ordering, and switch to git-based clean operations that respect .gitignore patterns while preserving development environments. Consolidate CI workflow test steps by integrating example tests into the test-ci target, eliminating redundant workflow steps. Exclude examples/srd_sn/sndata from all code quality tools (Black, Flake8, Mypy, Pylint) to avoid processing generated data files. Update .gitignore to remove obsolete cosmosis-standard-library entry and add .ruff_cache for the new linter.

Remove -j flags from make command examples in CONTRIBUTING.md, as the Makefile now handles parallelization automatically by default. Update the tip section to clarify how to override parallel execution behavior (use make -j1 for serial execution or JOBS=N to set a specific job count). Expand the target relationship diagram with additional dependencies and styling: add lint subtargets (black, flake8, mypy, pylint), documentation build steps (docs-build, api-docs, docs-generate-symbol-map), and intermediate test targets (unit-tests-pre, unit-tests-core). Add section comments to improve diagram readability.

Add a Parallelism Architecture section to explain Make and pytest parallel execution and where barriers apply. Include a mermaid diagram and key points so contributors can see which targets run sequentially versus in parallel.

Add clean-docs help entry to remind users how to remove generated documentation files. Enhance test-example target to preserve output in a temporary file when tests fail, making debugging easier. Adjust verbosity flags in test-example and test-integration from -vv to -v for cleaner output.

marcpaterno requested review from Copilot and vitenti and removed request for Copilot February 8, 2026 17:53

Copilot started reviewing on behalf of marcpaterno February 8, 2026 17:54 View session

Copilot AI review requested due to automatic review settings February 8, 2026 18:39

Copilot started reviewing on behalf of marcpaterno February 8, 2026 18:39 View session

Copilot AI reviewed Feb 8, 2026

View reviewed changes

marcpaterno marked this pull request as draft February 8, 2026 19:30

marcpaterno added 2 commits February 12, 2026 09:57

marcpaterno marked this pull request as ready for review February 15, 2026 01:01

marcpaterno added 5 commits February 15, 2026 10:05

Document build/test parallelism

4ee2d90

Add a Parallelism Architecture section to explain Make and pytest parallel execution and where barriers apply. Include a mermaid diagram and key points so contributors can see which targets run sequentially versus in parallel.

Merge branch 'master' into cleanup-makefile

0e89d88

Reset CACHE version to 0 in ci

3ffcab6

vitenti approved these changes Feb 24, 2026

View reviewed changes

marcpaterno added 2 commits February 24, 2026 13:55

Force use of /bin/bash in Makefile

cedcc38

Bump CACHE number to 15 to avoid failed caches

9bbb8b1

marcpaterno merged commit d3a8b87 into master Feb 24, 2026
13 checks passed

marcpaterno deleted the cleanup-makefile branch February 24, 2026 22:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modernize Makefile and CI workflow#615

Modernize Makefile and CI workflow#615
marcpaterno merged 11 commits intomasterfrom
cleanup-makefile

marcpaterno commented Feb 8, 2026 •

edited

Loading

Uh oh!

codecov bot commented Feb 8, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 8, 2026

Uh oh!

Copilot AI Feb 8, 2026

Uh oh!

Copilot AI Feb 8, 2026

Uh oh!

Copilot AI Feb 8, 2026

Uh oh!

Copilot AI Feb 8, 2026

Uh oh!

Copilot AI Feb 8, 2026

Uh oh!

Copilot AI Feb 8, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-pre-commit: format lint docs-verify test-ci ## Run all pre-commit checks
+pre-commit: ## Run all pre-commit checks
+	$(MAKE) format
+	$(MAKE) lint
+	$(MAKE) docs-verify
+	$(MAKE) test-ci

-test-all-coverage: unit-tests-pre unit-tests-core unit-tests-post ## Run core tests with coverage (fast)
-unit-tests-core:  ## Internal target for core tests with coverage
-	$(PYTEST) -vv --cov firecrown --cov-report xml --cov-branch -n auto
+test-all-coverage: ## Run core tests with coverage (fast)
+	$(MAKE) unit-tests-pre
+	$(MAKE) unit-tests-core
+	$(MAKE) unit-tests-post

	- name: Running example tests
	- name: Running example tests
	if: ${{ matrix.coverage == true }}

	4. Submit PR: Once your tests pass locally, submit a Pull Request to the `master` branch.
	4. Submit PR: Once your tests pass locally, submit a Pull Request to the `main` branch.

		JOBS ?= $(shell nproc 2>/dev/null \|\| sysctl -n hw.ncpu 2>/dev/null \|\| echo 4)
		MAKEFLAGS += -j$(JOBS)

Conversation

marcpaterno commented Feb 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

Checklist:

Uh oh!

codecov bot commented Feb 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

marcpaterno commented Feb 8, 2026 •

edited

Loading

codecov bot commented Feb 8, 2026 •

edited

Loading