-
Notifications
You must be signed in to change notification settings - Fork 1
Improve Claude Code prompting #2147
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
b19ad5c
Initial prompt for Claude Code
lucwollants a4c379e
Add rules about security
lucwollants 28fbf6f
Structure documentation inside the docs folder to make CLAUDE.md ligh…
lucwollants 17e985e
Only enable Claude review manually for now
lucwollants d218825
Move the review guidelines to a dedicated document
lucwollants cd985ac
Add guidelines about small commits and human review
lucwollants 84d4748
Remove meaningless comments
lucwollants 3f2fe83
Skip praise
lucwollants c4af643
Reference the doc folder
lucwollants cb1a733
Remove the steps AI already knows/does
lucwollants d440f40
Add a domain file to describe key concepts of UDB3
lucwollants 4533209
Add extra commands on how to execute features
lucwollants b274e86
Add extra URLs that can be used to verify results
lucwollants a7a7f80
Remove redundant data
lucwollants bd2f0bb
Rename refactor to features
lucwollants 05c9fa5
Split SAPI3 into specific files
lucwollants 2a1a6d3
Fix broken link
lucwollants 2e1e5f3
Manual review and cleanup
lucwollants f158c30
Merge branch 'master' into claude-code-prompt
lucwollants 1270576
Add guidelines for commenting code as less as possible
lucwollants b8e75da
Tweak coding guidelines order
lucwollants c4c547a
Add rule for naming exceptions
lucwollants 53609a0
Improve commit guidelines
lucwollants File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| # UDB3 Backend | ||
lucwollants marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| UiTdatabank 3 core application - an event-sourced backend built with Broadway. | ||
|
|
||
| ## Documentation | ||
|
|
||
| - [Domain](docs/domain.md) - Core domain concepts (Event, Place, Organizer, Offer) | ||
| - [Architecture](docs/architecture.md) - System design, tech stack, message flow | ||
| - [Coding Guidelines](docs/coding-guidelines.md) - Code style, conventions, security | ||
| - [Commands](docs/commands.md) - All available make commands | ||
| - [Review Guidelines](docs/review-guidelines.md) - PR review process and focus areas | ||
|
|
||
| ## Acceptance Tests | ||
|
|
||
| - [General](docs/features/general.md) - Acceptance test overview | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| # Architecture | ||
|
|
||
| UiTdatabank 3 (UDB3) is an event-sourced backend built with Broadway. | ||
|
|
||
| ## Tech Stack | ||
|
|
||
| - **Language**: PHP 8.1+ | ||
| - **Architecture**: Event Sourcing with [Broadway](https://github.com/broadway/broadway) | ||
| - **Database**: Doctrine DBAL | ||
| - **Message Broker**: RabbitMQ (AMQP) | ||
| - **Search**: SAPI3 (external Elasticsearch-based service) | ||
| - **Cache**: Redis | ||
| - **Testing**: PHPUnit (unit), Behat (acceptance) | ||
| - **Quality**: PHPStan (static analysis), PHP-CS-Fixer (code style) | ||
| - **Environment**: Docker with docker-compose | ||
|
|
||
| ## Project Structure | ||
|
|
||
| ``` | ||
| ├── app/ # Service providers and application wiring | ||
| ├── src/ # Core domain code | ||
| ├── tests/ # PHPUnit tests (mirrors src/ structure) | ||
| ├── features/ # Behat acceptance tests | ||
| ├── docker/ # Docker configuration | ||
| ├── docs/ # Documentation | ||
| │ └── refactor/ # Refactoring plans and progress | ||
| └── vendor/ # Dependencies (never modify) | ||
| ``` | ||
|
|
||
| ## Event Sourcing with Broadway | ||
|
|
||
| - **Aggregates**: Domain objects that emit events (in `src/`) | ||
| - **Projectors**: Build read models from events | ||
| - **Event Bus**: Distributes events to handlers | ||
|
|
||
| ## Message Flow: Domain Events to Search Indexing | ||
|
|
||
| ``` | ||
| 1. API Request (e.g., Create Event) | ||
| ↓ | ||
| 2. Command Handler processes command | ||
| ↓ | ||
| 3. Domain event created (e.g., EventCreated) | ||
| ↓ | ||
| 4. EventBus publishes event to subscribers | ||
| ↓ | ||
| 5. Projectors generate JSON-LD → emit *ProjectedToJSONLD events | ||
| ↓ | ||
| 6. AMQPPublisher sends message to RabbitMQ | ||
| ↓ | ||
| 7. External SAPI3 consumes message and indexes in Elasticsearch | ||
| ``` | ||
lucwollants marked this conversation as resolved.
Show resolved
Hide resolved
lucwollants marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| ### AMQP Configuration | ||
|
|
||
| - **Exchange**: `udb3.x.domain-events` | ||
| - **Routing**: Messages routed to `api`, `cli`, or `related` queues based on context | ||
| - **Message Types**: | ||
| - `application/vnd.cultuurnet.udb3-events.event-projected-to-jsonld+json` | ||
| - `application/vnd.cultuurnet.udb3-events.place-projected-to-jsonld+json` | ||
| - `application/vnd.cultuurnet.udb3-events.organizer-projected-to-jsonld+json` | ||
|
|
||
| ### SAPI3 Integration | ||
|
|
||
| SAPI3 is an external search service (Elasticsearch-based) that: | ||
| - Consumes messages from RabbitMQ | ||
| - Indexes events, places, and organizers | ||
| - Provides search endpoints proxied by UDB3 | ||
|
|
||
| **Configuration** (in `config.php`): | ||
| - Base URL: `http://search.uitdatabank.local:9000` | ||
| - ES URL: `http://search.uitdatabank.local:9200/` | ||
| - API Key: configured per environment | ||
|
|
||
| **Search Endpoints** (proxy to SAPI3): | ||
| - `/events` - Search events | ||
| - `/places` - Search places | ||
| - `/organizers` - Search organizers | ||
| - `/offers` - Search events and places combined | ||
|
|
||
| ## Docker Services | ||
|
|
||
| | Service | Port | Purpose | | ||
| |---------|------|---------| | ||
| | PHP | 80 | Application | | ||
| | MySQL | 3306 | Database | | ||
| | Redis | 6379 | Cache | | ||
| | RabbitMQ | 5672, 15672 | Message broker (AMQP, Management UI) | | ||
| | Mailpit | 1025, 8025 | Email testing (SMTP, Web UI) | | ||
lucwollants marked this conversation as resolved.
Show resolved
Hide resolved
lucwollants marked this conversation as resolved.
Show resolved
Hide resolved
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,118 @@ | ||
| # Coding Guidelines | ||
|
|
||
| ## General | ||
|
|
||
| - Use `final` classes by default | ||
| - Follow PSR-4 autoloading under `CultuurNet\UDB3\` namespace | ||
| - Keep classes small and focused (Single Responsibility) | ||
|
|
||
| ## PHP 8.1 Features | ||
|
|
||
| Use **readonly properties** with constructor promotion for immutable class properties. | ||
|
|
||
| See `src/Doctrine/DBALDatabaseConnectionChecker.php` for an example. | ||
|
|
||
| > **Note**: Do NOT use PHP 8.2 `readonly class` syntax - we're on PHP 8.1. | ||
lucwollants marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| ## Comments & Self-Documenting Code | ||
|
|
||
| Avoid comments in favor of self-documenting code: | ||
|
|
||
| - **Type hints**: Use typed parameters, typed properties and return types instead of docblock descriptions | ||
| - **Method names**: Use descriptive, self-explaining method and function names | ||
| - **Variable names**: Choose clear, meaningful variable names that convey purpose | ||
| - **Class names**: Use descriptive class names that explain their responsibility | ||
| - **File names**: Mirror class names for easy discoverability | ||
|
|
||
| Comments are acceptable when explaining **why** something is done (not **what**), such as workarounds for external limitations or non-obvious business rules. | ||
|
|
||
| ## Naming | ||
|
|
||
| ### Interfaces | ||
|
|
||
| - Descriptive name without `Interface` suffix (e.g., `DatabaseConnectionChecker`) | ||
| - Implementations: Prefix with technology (e.g., `DBALDatabaseConnectionChecker`) | ||
|
|
||
| ### Exceptions | ||
|
|
||
| - Descriptive name without `Exception` suffix | ||
| - Name should describe what went wrong (e.g., `DocumentDoesNotExist`, `NewsArticleNotFound`) | ||
|
|
||
| ## Testing | ||
|
|
||
| - Use `@test` annotation style for test methods | ||
| - Mock objects use intersection types: `Connection&MockObject` | ||
| - Prefer separate test methods over data providers for clarity | ||
| - Test file location mirrors source: `src/Foo/Bar.php` → `tests/Foo/BarTest.php` | ||
|
|
||
| ## Gotchas & Pitfalls | ||
|
|
||
| | Issue | Solution | | ||
| |-------|----------| | ||
| | Commands fail outside Docker | Use `make` commands, not direct `composer` calls | | ||
| | `withConsecutive` deprecation | Avoid in new tests - use separate test methods | | ||
| | Uppercase `String` type hint | Legacy code - don't "fix" these | | ||
| | Tests fail after changes | Run `make ci` before committing | | ||
|
|
||
| ## Restrictions | ||
|
|
||
| - **Never** modify files in `vendor/` | ||
| - **Never** commit `.env` or credentials files | ||
| - **Avoid** creating new service providers - extend existing ones in `app/` | ||
| - **Avoid** adding dependencies without team discussion | ||
| - **Always** run `make ci` before considering work complete | ||
|
|
||
| ## AI Restrictions | ||
|
|
||
| - **Never** create commits - only humans commit code | ||
lucwollants marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| - **Plan** larger changes but implement step by step, waiting for human review between steps | ||
|
|
||
| ## Dependency Injection | ||
|
|
||
| - Service providers in `app/` wire all dependencies | ||
| - Use **constructor injection**, not service locators | ||
| - Extract testable logic into separate classes with **interfaces** | ||
|
|
||
| See `src/Doctrine/DatabaseConnectionChecker.php` and `src/Doctrine/DBALDatabaseConnectionChecker.php` for an example of interface + implementation pattern. | ||
|
|
||
| ## Workflow | ||
|
|
||
| - **Small commits**: One logical change per commit | ||
| - A logical change can span multiple files (e.g., source code + tests, or a refactor touching several related files) | ||
| - All changes in the commit should belong together and serve a single purpose | ||
| - The goal is to make commits easy to review and understand | ||
| - **Small pull requests**: Easier to review, faster to merge | ||
| - **Feature flags**: Deploy often, enable features when ready | ||
|
|
||
| ## Preferences | ||
|
|
||
| When working on this codebase, prefer: | ||
|
|
||
| - Extracting logic into testable classes with interfaces over inline implementation | ||
| - Constructor injection over service locator patterns | ||
| - Explicit code over clever abstractions | ||
| - Integration with existing patterns over introducing new ones | ||
|
|
||
| ## Security | ||
lucwollants marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| ### Never Commit | ||
|
|
||
| - `.env` files or environment-specific configurations | ||
| - API keys, tokens, or credentials | ||
| - Private keys or certificates | ||
| - Database connection strings with passwords | ||
| - Any hardcoded secrets | ||
|
|
||
| ### Configuration | ||
|
|
||
| - Secrets belong in environment variables, not in code | ||
| - Use `.env.example` or `.env.dist` for documenting required env vars (without actual values) | ||
| - Configuration files with secrets should be in `.gitignore` | ||
|
|
||
| ### Secure Coding | ||
|
|
||
| - No hardcoded credentials or secrets in code | ||
| - No sensitive data in log statements | ||
lucwollants marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| - Validate input on user-provided data | ||
| - SQL queries use parameterized statements (handled by Doctrine DBAL) | ||
| - No sensitive data exposure in error messages | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| # Commands | ||
|
|
||
| > **Important**: Always use `make` commands, not direct `composer` or `vendor/bin` calls. The project runs in Docker. | ||
lucwollants marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| ## Environment Management | ||
|
|
||
| ```bash | ||
| make up # Start containers | ||
| make down # Stop containers | ||
| make bash # Shell into PHP container | ||
| make install # Install composer dependencies | ||
| make migrate # Run database migrations | ||
| make init # Install + migrate (fresh setup) | ||
| ``` | ||
|
|
||
| ## Quality Assurance | ||
|
|
||
| ```bash | ||
| make ci # Run ALL checks (PHPStan + tests + code style) | ||
| make stan # PHPStan static analysis only | ||
| make test # PHPUnit tests only | ||
| make cs # Code style check (dry-run) | ||
| make cs-fix # Auto-fix code style issues | ||
| ``` | ||
|
|
||
| ## Unit Testing | ||
|
|
||
| ```bash | ||
| make test # All unit tests | ||
| make test-filter filter=EventBusForwarding # Tests matching filter | ||
| make test-group group=integration # Tests in specific group | ||
| ``` | ||
|
|
||
| ## Acceptance Testing (Behat) | ||
|
|
||
| ```bash | ||
| make feature # All feature tests | ||
| make feature-tag tag=@events # Feature tests by tag | ||
| make feature-tag tag=@sapi3 # SAPI3 search integration tests | ||
| make feature-filter path=features/search/auth.feature # All scenarios in a file | ||
| make feature-filter path=features/search/auth.feature:25 # Single scenario by line number | ||
| ``` | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| # Domain Concepts | ||
|
|
||
| ## Resources | ||
|
|
||
| UDB3 manages three main resources: | ||
|
|
||
| | Resource | Description | | ||
| |----------|-------------| | ||
| | **Event** | An activity happening at a specific time and place | | ||
| | **Place** | A location where events can happen | | ||
| | **Organizer** | The entity organizing events | | ||
|
|
||
| ## Offers | ||
|
|
||
| Event and Place are collectively referred to as **Offer**. This terminology is used throughout the codebase and APIs. |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.