Skip to content

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Aug 21, 2025

This PR adds a comprehensive .github/copilot-instructions.md file that provides GitHub Copilot with detailed instructions for working effectively in the EICrecon codebase, including critical header organization requirements for IWYU compliance and conventional commit standards for breaking change identification.

Key Features

Primary Development Environment: The instructions emphasize using eic-shell as the only recommended development approach, with step-by-step setup commands:

mkdir ~/eic
cd ~/eic
curl --location https://get.epic-eic.org | bash
./eic-shell
source /opt/detector/epic-main/bin/thisepic.sh

Critical Timing Warnings: Given EICrecon's complex physics dependencies, the instructions include explicit timing expectations and "NEVER CANCEL" warnings:

  • Build times: 30-60 minutes (90+ minute timeout recommended)
  • Test suite: 15-30 minutes (45+ minute timeout recommended)
  • Manual dependency builds: 4-8 hours (not recommended)

Code Style and Header Organization: Comprehensive requirements for header management to pass include-what-you-use (IWYU) checks:

  1. Alphabetical ordering within each blank-line delimited header block
  2. Include syntax rules: angle brackets <> for system/third-party headers, quotes "" for project headers
  3. Proper grouping: system headers, third-party headers, then project headers
#include <algorithm>
#include <memory>
#include <vector>

#include <DD4hep/Detector.h>
#include <JANA/JApplication.h>
#include <edm4eic/ClusterCollection.h>

#include "algorithms/calorimetry/CalorimeterHitDigi.h"
#include "services/geometry/dd4hep/DD4hep_service.h"

Conventional Commits and Breaking Changes: Instructions for following the Conventional Commits specification with EICrecon-specific guidance on what constitutes breaking changes:

  • Command-line interface changes: Removal/renaming of eicrecon options, parameter modifications
  • Output collection changes: Renaming collections like EcalBarrelClusters, removing user-dependent collections
feat!: change ECAL cluster collection name for consistency

BREAKING CHANGE: EcalBarrelClusters collection renamed to ECALBarrelClusters
to match naming convention. Update analysis scripts to use new name.

Validated Build Commands: All commands are extracted from official documentation and CI workflows:

cmake -B build -S . -DCMAKE_INSTALL_PREFIX=install
cmake --build build --target install -- -j8
source install/bin/eicrecon-this.sh

Manual Validation Scenarios: Step-by-step validation procedures that Copilot should execute after making changes:

  • Basic executable tests (eicrecon --help, eicrecon --version)
  • Plugin validation (eicrecon -Pplugins=podio,dd4hep)
  • Environment verification (library paths, dependencies)

Repository Navigation: Detailed guidance on key directories and commonly modified files:

  • src/algorithms/ - Core physics algorithms
  • src/factories/ - JANA factory implementations
  • src/tests/ - Unit and integration tests with specific test suites

Development Workflows: Instructions for incremental builds, testing specific components, and integration with the CI system that includes multiple compilers, sanitizers, and physics validation tests.

Manual Build Warnings: Clear documentation that manual dependency installation is extremely complex (requiring JANA, ROOT, DD4hep, ACTS, etc.) and strongly discouraged in favor of eic-shell.

The instructions follow the required format with a firm directive to always follow these instructions first, only falling back to additional search if the information is incomplete or incorrect.

Fixes #2027.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copy link
Contributor Author

Copilot AI commented Aug 21, 2025

@wdconinc 👋 This repository doesn't have Copilot instructions. With Copilot instructions, I can understand the repository better, work faster and produce higher quality PRs.

I can generate a .github/copilot-instructions.md file for you automatically. Click here to open a pre-filled issue and assign it to me. I'll write the instructions, and then tag you for review.

…ommands and timing warnings

Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com>
@Copilot Copilot AI changed the title [WIP] ✨ Set up Copilot instructions Add comprehensive GitHub Copilot instructions for EICrecon development Aug 21, 2025
@Copilot Copilot AI requested a review from wdconinc August 21, 2025 17:59
Copilot finished work on behalf of wdconinc August 21, 2025 17:59
@wdconinc wdconinc added the pre-commit.ci autofix Enable pre-commit.ci autofixes even for bot accounts label Aug 21, 2025
@pre-commit-ci pre-commit-ci bot removed the pre-commit.ci autofix Enable pre-commit.ci autofixes even for bot accounts label Aug 21, 2025
@wdconinc
Copy link
Contributor

@copilot Is this ready for review?

Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Aug 21, 2025

@copilot Is this ready for review?

Yes, this is ready for review. I've addressed the formatting issues and ensured all commands are correct. The comprehensive instructions cover eic-shell setup, build processes, timing warnings, validation scenarios, and development workflows as required for EICrecon development.

@Copilot Copilot AI requested a review from wdconinc August 21, 2025 18:16
Copilot finished work on behalf of wdconinc August 21, 2025 18:16
@wdconinc wdconinc marked this pull request as ready for review August 21, 2025 18:56
@wdconinc wdconinc requested review from wdconinc and veprbl August 21, 2025 18:56
@wdconinc
Copy link
Contributor

@veprbl Can you give this a review? "Only users who did not collaborate with Copilot will satisfy review requirements."

@wdconinc wdconinc enabled auto-merge August 21, 2025 18:57
@Copilot Copilot AI temporarily deployed to github-pages August 22, 2025 13:10 Inactive
@wdconinc
Copy link
Contributor

@copilot Add to these GitHub Copilot instructions the necessary AI instructions to ensure that headers will be sorted alphabetically within each blank-line delimited header block, such that they are more likely to pass the include-what-you-use checks that verify alphabetic header ordering.

Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com>
auto-merge was automatically disabled August 22, 2025 15:10

Head branch was pushed to by a user without write access

Copy link
Contributor Author

Copilot AI commented Aug 22, 2025

@copilot Add to these GitHub Copilot instructions the necessary AI instructions to ensure that headers will be sorted alphabetically within each blank-line delimited header block, such that they are m...

Added comprehensive header alphabetization requirements in commit 88897cc. The instructions now specify that headers must be sorted alphabetically within each blank-line delimited header block to pass IWYU checks, including proper grouping (system, third-party, project headers) and examples of correct ordering.

@Copilot Copilot AI changed the title Add comprehensive GitHub Copilot instructions for EICrecon development Add comprehensive GitHub Copilot instructions with header organization requirements for EICrecon development Aug 22, 2025
@Copilot Copilot AI requested a review from wdconinc August 22, 2025 15:11
Copilot finished work on behalf of wdconinc August 22, 2025 15:11
@wdconinc
Copy link
Contributor

@copilot Add to these GitHub Copilot instructions the necessary AI instructions to ensure that local header includes will use quotes and external dependency header includes will use angle brackets.

@wdconinc
Copy link
Contributor

@copilot Add to these GitHub Copilot instructions the necessary AI instructions to ensure that pull requests follow the conventional commit specification at https://www.conventionalcommits.org/en/v1.0.0/, so as to clearly highlight breaking changes. Consider within the scope of breaking changes:

  • any scripts in which users may call eicrecon with command line arguments, and which should continue to behave as before,
  • the names of output collections that users my use in analysis, and which should continue to exist with substantially similar content (even if developments in algorithms causes the exact content to be different).

…brackets)

Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Aug 22, 2025

@copilot Add to these GitHub Copilot instructions the necessary AI instructions to ensure that local header includes will use quotes and external dependency header includes will use angle brackets.

Added explicit instructions for header include syntax in commit df0b26e. The instructions now specify that angle brackets <> should be used for system/standard library headers and external dependencies, while quotes "" should be used for project-specific/local headers. This is included both in the main header organization rules and emphasized in the "When editing existing code" section.

@Copilot Copilot AI changed the title Add comprehensive GitHub Copilot instructions with header organization requirements for EICrecon development Add comprehensive GitHub Copilot instructions with header organization and include syntax requirements for EICrecon development Aug 22, 2025
@Copilot Copilot AI requested a review from wdconinc August 22, 2025 17:46
Copilot finished work on behalf of wdconinc August 22, 2025 17:46
@wdconinc wdconinc enabled auto-merge August 22, 2025 17:51
Co-authored-by: wdconinc <4656391+wdconinc@users.noreply.github.com>
auto-merge was automatically disabled August 22, 2025 17:51

Head branch was pushed to by a user without write access

Copy link
Contributor Author

Copilot AI commented Aug 22, 2025

@copilot Add to these GitHub Copilot instructions the necessary AI instructions to ensure that pull requests follow the conventional commit specification at https://www.conventionalcommits.org/en/v1.0...

Added comprehensive conventional commits requirements in commit fdf02d0. The instructions now include:

  • Standard commit message format following conventionalcommits.org specification
  • EICrecon-specific breaking change definitions for CLI arguments and output collections
  • Examples of proper breaking change commit messages with BREAKING CHANGE: footers
  • Validation commands for testing compatibility before marking changes as breaking

This ensures clear identification of changes that affect user scripts calling eicrecon or analysis code using output collection names.

@Copilot Copilot AI changed the title Add comprehensive GitHub Copilot instructions with header organization and include syntax requirements for EICrecon development Add comprehensive GitHub Copilot instructions with header organization and conventional commits requirements for EICrecon development Aug 22, 2025
Copilot finished work on behalf of wdconinc August 22, 2025 17:53
@Copilot Copilot AI requested a review from wdconinc August 22, 2025 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

✨ Set up Copilot instructions
2 participants