fix: rewrite CONTRIBUTING.md with accurate workflow

[Kamilbenkirane]

Summary

• Template-first: contributors must copy from templates/, not greenfield
• Explicit provider addition checklist covering both API layer and modality wiring
• Three tiers: small fixes (PR directly), additive changes (issue first), core semantics (maintainer approval required)
• Added missing steps: PROVIDERS dict registration, model registration, provider export
• Documented pre-commit vs pre-push hook behavior
• Listed all make targets

Closes #57

Test plan

• make ci passes
• All referenced paths/commands verified against the codebase

[claude]

PR Review: CONTRIBUTING.md Rewrite

Summary

This PR successfully addresses issue #57 by creating a comprehensive CONTRIBUTING.md file. The documentation is well-structured, accurate, and provides clear guidance for contributors. Overall, this is a strong addition to the project.

Strengths

Excellent Structure

• Clear TL;DR section gets contributors oriented quickly
• Logical flow from setup → local checks → workflow → specific tasks
• Three-tier contribution model (small fixes, additive changes, core semantics) is well-defined
• Template-first approach is emphasized appropriately

Accurate Technical Details

• All referenced paths verified against codebase:
  • templates/ directory exists with correct structure
  • src/celeste/core.py contains Provider enum ✓
  • src/celeste/modalities/<modality>/providers/__init__.py has PROVIDERS dict ✓
  • tests/unit_tests/test_provider_api_templates.py exists ✓
  • src/celeste/providers/api_references.md exists ✓
• Make targets match Makefile exactly
• Pre-commit/pre-push hook behavior documented correctly

Comprehensive Provider Addition Guide

The 11-step checklist for adding providers is thorough and covers both:

• Provider API layer (steps 1-5)
• Modality wiring (steps 6-11)

This addresses a critical gap in contribution documentation.

Issues Found

Critical: Typo on Line 1

- a# Contributing to Celeste
+ # Contributing to Celeste

There's an extra "a" character before the heading. This will break Markdown rendering.

Minor: Template Path Discrepancy

Line 53 states:

copy from templates/providers/, place in src/celeste/providers/<provider>/

However, the actual template structure is:

templates/providers/{provider_slug}/src/celeste_{provider_slug}/{api_slug}/

The templates are NOT directly in the provider directory but in a nested src/celeste_{provider_slug} structure.

Recommendation: Clarify that contributors need to:

1. Copy the entire templates/providers/{provider_slug}/ directory
2. Rename celeste_{provider_slug} to match their provider
3. Place the contents in src/celeste/providers/<provider>/

The same applies to line 61 for modalities - the template path is templates/modalities/{modality_slug}/src/celeste_{modality_slug}/ not directly templates/modalities/.

Minor: Setup Command Incomplete

Line 15 says:

uv sync --group dev

But based on pyproject.toml, the project uses dependency groups. The command should specify all necessary groups or match the actual setup requirements. Consider whether --all-groups or specific groups beyond dev are needed.

Suggestion: Add Testing Context

The provider addition section mentions make test will catch template contract violations, but it would be helpful to mention:

• What specific errors to expect if templates aren't followed correctly
• That contributors should run make ci after each step to validate their work incrementally

Security Considerations

✅ No security concerns - the document correctly:

• References environment variable handling for API keys
• Directs security issues to private channels
• Doesn't expose any sensitive information

Test Coverage

✅ The PR includes a test plan showing make ci passes
✅ All referenced paths and commands verified

Performance Considerations

N/A - Documentation only

Code Quality & Best Practices

✅ Writing is clear and concise
✅ Examples are practical
✅ Tone is welcoming and professional
✅ Good use of emphasis for critical requirements (bold, code blocks, quotes)

Recommendations

Must Fix

1. Remove the "a" typo on line 1

Should Consider

2. Clarify template directory structure and the copy/rename process
3. Verify the uv sync --group dev command is sufficient or if additional groups are needed
4. Add a brief note about what template contract violations look like when running make test

Nice to Have

5. Consider adding a "Quick Reference" section for the most common contribution workflows
6. Maybe add an example of a minimal provider addition PR for reference

Verdict

Approve with minor fixes - This PR significantly improves the contributor experience and accurately documents the workflow. The typo on line 1 must be fixed before merge, but otherwise this is excellent work that properly closes issue #57.

Great job on creating comprehensive, accurate documentation! 🎉

[claude]

Code Review for PR #131

Summary

This PR creates a comprehensive CONTRIBUTING.md file to address issue #57. The document provides clear guidance for contributors on setup, development workflow, and adding new providers.

✅ Strengths

1. Comprehensive Coverage: The document addresses all key areas mentioned in issue Missing CONTRIBUTING.md file referenced in README #57 - setup, testing, provider addition workflow, and PR process.

2. Template-First Approach: The emphasis on using templates (line 50-51, 59) aligns perfectly with the existing codebase architecture. The reference to tests/unit_tests/test_provider_api_templates.py (line 58) is accurate and provides enforcement.

3. Accurate Technical Details:

   • All Makefile targets referenced (lines 26-31) match the actual Makefile
   • Pre-commit and pre-push hook behavior is correctly documented (lines 17-20)
   • The 11-step provider addition checklist is comprehensive and matches actual code patterns

4. Tiered Contribution Model: The three-tier approach (lines 43-46) is sensible - small fixes → additive changes → core semantics - providing clear guidance on when to open issues first.

5. Security Contact: Proper vulnerability reporting guidance (lines 82-83) follows best practices.

🐛 Issues Found

Critical Issue: Typo in First Line

Location: Line 1
Issue: a# Contributing to Celeste has an extra "a" before the heading marker
Impact: This will render incorrectly in GitHub's markdown viewer
Fix: Should be # Contributing to Celeste

-a# Contributing to Celeste
+# Contributing to Celeste

💡 Suggestions for Enhancement

1. Model Registration Step: While step 9 mentions "Register models in src/celeste/modalities/<modality>/models.py", it could be clearer. Looking at src/celeste/modalities/text/models.py, contributors need to:

   • Import the provider's MODELS constant
   • Add it to the MODELS list

   Consider adding a concrete example.

2. PROVIDERS Dict Registration: Step 10 correctly mentions the PROVIDERS dict, which is verified in src/celeste/modalities/text/providers/__init__.py:18-30. This is good.

3. Provider Export: Step 3 mentions exporting in src/celeste/providers/__init__.py - verified correct at src/celeste/providers/__init__.py:3-19,21-37.

4. Missing from Checklist: The Provider enum registration (step 1) should clarify that it's in src/celeste/core.py:6-29. The current wording "(if new)" is good but could reference the specific file location.

5. Pre-commit Commands: Consider noting that contributors can run pre-commit run --all-files to manually trigger all hooks before committing.

6. Integration Test Clarification: Line 44 mentions "new modalities" should have an issue first, but integration tests for new modalities might need additional setup beyond just API keys. Consider adding a note about this.

📊 Test Coverage Assessment

✅ The document references the template contract test at line 58, which exists at tests/unit_tests/test_provider_api_templates.py
✅ Pre-push hooks include pytest with coverage (verified in .pre-commit-config.yaml)
✅ make ci runs the full pipeline including tests

🔒 Security Review

✅ Proper vulnerability reporting process documented
✅ No sensitive information exposed
✅ Bandit security scanning mentioned in the workflow
✅ Environment variable pattern (.env.example) correctly referenced

⚡ Performance Considerations

N/A - This is documentation only, no performance implications.

📋 Best Practices

✅ Clear, concise writing
✅ Proper markdown formatting (except the line 1 typo)
✅ Logical organization with clear sections
✅ Accurate technical references to actual code paths
✅ Includes both quick start (TL;DR) and detailed sections

🎯 Recommendation

Approval with one required fix: Fix the typo on line 1, then this is ready to merge.

The checklist is comprehensive and accurate. The template-first approach is well-documented. The only blocking issue is the markdown syntax error that will affect rendering.

Test Plan Verification

✅ The author reports make ci passes - appropriate for documentation changes
✅ All referenced paths verified against codebase
✅ All commands tested for accuracy

---

Verdict: Approve after fixing the line 1 typo. Excellent work creating comprehensive contribution guidelines! 🎉

[claude]

Pull Request Review: CONTRIBUTING.md

Overview

This PR adds a comprehensive CONTRIBUTING.md file that was previously missing from the repository. The document provides clear guidance for contributors and closes issue #57.

✅ Strengths

1. Excellent Structure & Clarity

• Clear TL;DR at the top for quick reference
• Well-organized sections with logical flow
• Template-first approach is emphasized appropriately
• Good use of code blocks and formatting

2. Accurate Technical Details

• ✅ All make targets verified against Makefile (ci, test, lint, format, typecheck, security)
• ✅ Pre-commit vs pre-push hook behavior correctly documented (matches .pre-commit-config.yaml)
• ✅ Template structure accurately reflects actual templates/ directory
• ✅ Provider/modality registration steps match actual codebase patterns
• ✅ Test validation reference (test_provider_api_templates.py) is accurate

3. Comprehensive Provider Addition Guide

The 11-step provider addition checklist is thorough and aligns with the codebase:

• Provider enum registration in core.py
• Auth registration via register_auth()
• Export in src/celeste/providers/init.py
• API layer scaffolding (config, client, parameters, streaming)
• Modality wiring (models, mappers, client)
• Model registration in modality models.py
• PROVIDERS dict registration ✅
• API reference documentation

4. Smart Contribution Tiers

The three-tier approach is practical:

• Small fixes: direct PR
• Additive changes: issue first
• Core semantics: maintainer approval required

This prevents wasted effort and ensures alignment.

📋 Suggestions & Minor Issues

1. Integration Test Command Clarification

Line 38: make integration-test

The Makefile shows this runs with -m integration flag and pytest-xdist. Consider adding a note about expected test runtime and that tests run in parallel by default.

2. Test Coverage Threshold

Line 31: make ci runs tests

The Makefile shows --cov-fail-under=80 but this isn't mentioned in the contributing guide. Contributors should know their changes need to meet the 80% coverage threshold.

Suggested: make test # Unit tests with 80% coverage threshold required

3. Security Contact Enhancement

Lines 82-83: Email contact for security vulnerabilities

Consider emphasizing GitHub's private vulnerability reporting as the preferred method, with email as backup.

4. Template Contract Testing

Line 59: References tests/unit_tests/test_provider_api_templates.py

This is accurate and helpful. Consider adding what happens when the contract is violated to help contributors understand error messages.

🔍 Testing & Validation

Verified Against Codebase:

• ✅ All file paths exist and are correct
• ✅ Make targets match Makefile
• ✅ Pre-commit hooks configuration accurate
• ✅ Template structure matches templates/ directory
• ✅ Provider registration steps match actual code patterns
• ✅ .env.example file exists as referenced
• ✅ uv is the documented package manager
• ✅ Test file references are accurate

PR Metadata:

• ✅ Test plan completed (make ci passes)
• ✅ Closes issue Missing CONTRIBUTING.md file referenced in README #57
• ✅ Clear summary in PR description
• ✅ Only adds new file (no modifications to existing code)

🎯 Verdict

APPROVE ✅

This is an excellent addition to the repository. The CONTRIBUTING.md is:

• Accurate and technically correct
• Well-structured and easy to follow
• Comprehensive without being overwhelming
• Properly validated against the actual codebase

The suggestions above are minor enhancements that could be addressed in follow-up PRs if desired. None are blocking issues.

Recommendations:

1. Merge as-is - the document is production-ready
2. Consider the suggestions above for a future iteration
3. Update the guide when new modalities or significant workflow changes are introduced

Great work! This will significantly improve the contributor experience. 🚀

---

Review performed with verification against: Makefile, .pre-commit-config.yaml, pyproject.toml, templates/, src/celeste/ structure, and tests/