Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 3, 2025

Configures .github/AGENTS.md to provide AI coding agents (including GitHub Copilot) with repository-specific context and conventions.

Contents

  • Project structure: Python package in python/egglog/, Rust PyO3 bindings in src/, Sphinx docs
  • Build toolchain: uv package manager (uv sync --all-extras), version references in config files
  • Code standards: Ruff (120 char lines, specific rule exceptions), mypy type checking, syrupy snapshot testing
  • Python file organization: Detailed overview of all core Python modules and the public API export pattern
  • Development workflow: Test commands, build commands, changelog requirements
  • Contribution patterns:
    • E-graph API design (inheriting from egglog.Expr)
    • Value retrieval and pattern matching
    • Parallelism configuration
    • Rust-Python type conversions

The instructions enable agents to suggest code that follows existing patterns, uses correct tooling commands, respects the hybrid Python/Rust architecture, and understands the public API export structure.

Fixes #370

Original prompt

This section details on the original issue you should resolve

<issue_title>✨ Set up Copilot instructions</issue_title>
<issue_description>Configure instructions for this repository as documented in Best practices for Copilot coding agent in your repository.

</issue_description>

Comments on the Issue (you are @copilot in this section)


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

Copilot AI changed the title [WIP] ✨ Set up Copilot instructions Add GitHub Copilot instructions for repository context Nov 3, 2025
Copilot AI requested a review from saulshanabrook November 3, 2025 04:28
Copilot finished work on behalf of saulshanabrook November 3, 2025 04:28
Copy link
Member

@saulshanabrook saulshanabrook left a comment

Choose a reason for hiding this comment

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

@copilot

Can you also rename this to AGENTS.md so that it is used for other agents as well?

Can you also go through the docs and make sure you aren't missing anything and also look at the past 20 commits on main to see if you are missing any instructions?

Comment on lines 25 to 26
- **Rust toolchain** - Version 1.79.0 (specified in rust-toolchain.toml)
- **Python** - 3.10+ (see .python-version)
Copy link
Member

Choose a reason for hiding this comment

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

Can you not keep the versions in here so I don't have to update this when they change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed version numbers. See commit dd6db0e.


```bash
# Install dependencies
uv sync --extra dev --locked
Copy link
Member

Choose a reason for hiding this comment

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

Can you change this to uv sync --all-extras

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to uv sync --all-extras. See commit dd6db0e.

Comment on lines +92 to +100
- `src/lib.rs` - Main library entry point
- `src/egraph.rs` - E-graph implementation
- `src/conversions.rs` - Type conversions between Python and Rust
- `src/py_object_sort.rs` - Python object handling
- `src/extract.rs` - Extraction functionality
- `src/error.rs` - Error handling
- `src/serialize.rs` - Serialization support
- `src/termdag.rs` - Term DAG operations
- `src/utils.rs` - Utility functions
Copy link
Member

Choose a reason for hiding this comment

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

Can you add overviews of the Python files as well?

And document that the public python interface is all from egglog so anything that is public should be exported at the top level.

There is also the lower level egglog.bindings module which can also be used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added Python file overviews and documented that all public APIs should be exported at the top level in __init__.py. Also documented egglog.bindings for lower-level access. See commit dd6db0e.

Comment on lines 122 to 127
### Python Dependencies
- **Core**: typing-extensions, black, graphviz, anywidget
- **Array support**: scikit-learn, array_api_compat, numba, numpy>2
- **Dev tools**: ruff, pre-commit, mypy, jupyterlab
- **Testing**: pytest, pytest-benchmark, syrupy (inline snapshots)
- **Docs**: sphinx and related packages
Copy link
Member

Choose a reason for hiding this comment

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

Remove this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed Pre-commit Hooks section. See commit dd6db0e.

Comment on lines 129 to 133
### Rust Dependencies
- **PyO3**: Python bindings framework
- **egglog**: Core e-graph library
- **egraph-serialize**: Serialization support
- **serde_json**: JSON handling
Copy link
Member

Choose a reason for hiding this comment

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

Remove this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed Dependencies section. See commit dd6db0e.

1. Update or add tests in `python/tests/` for Python changes
2. Run the full test suite before committing
3. Ensure type checking passes with `make mypy`
4. Build documentation if changing public APIs
Copy link
Member

Choose a reason for hiding this comment

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

Add changelog entry

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added changelog entry requirement to contributing guidelines. See commit dd6db0e.

## Common Patterns

### Python API Design
- Use `@egraph.class_` decorator for e-graph classes
Copy link
Member

Choose a reason for hiding this comment

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

This is depracted, now inherit from egglog.Expr

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed to use Expr inheritance instead of deprecated @egraph.class_ decorator. See commit dd6db0e.

- `version.yml` - Version management
- `update-changelog.yml` - Changelog automation

Tests run on Python 3.10, 3.11, 3.12, and 3.13.
Copy link
Member

Choose a reason for hiding this comment

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

Remove this to not have to keep up to date.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed CI section. See commit dd6db0e.

@codspeed-hq
Copy link

codspeed-hq bot commented Nov 3, 2025

CodSpeed Performance Report

Merging #371 will not alter performance

Comparing copilot/fix-71104912-561003658-37916b2b-13ae-4941-ad7d-463e42a7636c (33f1a3e) with main (9956e54)1

Summary

✅ 14 untouched

Footnotes

  1. No successful run was found on main (33f1a3e) during the generation of this report, so 9956e54 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@saulshanabrook
Copy link
Member

@copilot can you fix in response to the review?

Copy link
Contributor Author

Copilot AI commented Nov 3, 2025

@copilot can you fix in response to the review?

All review feedback addressed in commit dd6db0e:

  • Removed version numbers
  • Changed to uv sync --all-extras
  • Added Python file overviews and public interface docs
  • Removed Dependencies and Pre-commit Hooks sections
  • Added changelog requirement
  • Fixed decorator pattern to use Expr inheritance
  • Removed CI section

@copilot

Can you also rename this to AGENTS.md so that it is used for other agents as well?
...

Done! Renamed to AGENTS.md and reviewed docs/recent commits. Added Python file overviews, public interface documentation pattern, parallelism notes, and value retrieval patterns. See commit dd6db0e.

Copilot AI changed the title Add GitHub Copilot instructions for repository context Add agent instructions for repository context Nov 3, 2025
Copilot AI requested a review from saulshanabrook November 3, 2025 04:48
Copilot finished work on behalf of saulshanabrook November 3, 2025 04:48
@saulshanabrook
Copy link
Member

@codex can you review this PR and make any changes you think are useful for an agents file, also by reviewing the recent history of changes on this repo for best practices?

@chatgpt-codex-connector
Copy link

Summary

  • Documented the pinned toolchain versions, added commands for reinstalling the Rust extension, and noted how to refresh the bundled visualizer assets for agents working in this repository. .github/AGENTS.mdL25-L53
  • Expanded the Python package overview to highlight example and experimental modules and clarified that generated bindings should not be edited manually. .github/AGENTS.mdL119-L138
  • Updated the contributing guidance to align with the automated changelog workflow and explain when manual notes are still needed. .github/AGENTS.mdL151-L158

Testing

  • ⚠️ tests not run (documentation-only change)

View task →

@saulshanabrook saulshanabrook marked this pull request as ready for review November 3, 2025 05:09
@saulshanabrook saulshanabrook merged commit 40b1326 into main Nov 3, 2025
18 of 19 checks passed
@saulshanabrook saulshanabrook deleted the copilot/fix-71104912-561003658-37916b2b-13ae-4941-ad7d-463e42a7636c branch November 3, 2025 05:10
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.

✨ Set up Copilot instructions

2 participants