415 lines
15 KiB
Markdown
415 lines
15 KiB
Markdown
---
|
|
name: writing-prs-issues-commits
|
|
description: Guidelines for writing effective PR descriptions, GitHub issues, and git commit messages following the inverted pyramid style. Use only when you work on them.
|
|
---
|
|
|
|
# Writing Effective PRs, Issues, and Git Commit Messages
|
|
|
|
## Core Principle: Inverted Pyramid
|
|
|
|
All three artifacts (PRs, issues, commit messages) should follow the **inverted pyramid** style: the most important information comes first, with progressively finer details below.
|
|
|
|
> When scrolling through a commit history or PR list, readers need to quickly determine relevance. The "what" and "why" must be immediately visible.
|
|
|
|
---
|
|
|
|
## 1. Git Commit Messages
|
|
|
|
### Structure
|
|
|
|
```
|
|
<type>: <short summary (50 chars max)>
|
|
|
|
<blank line>
|
|
|
|
<one paragraph describing the change and its motivation>
|
|
|
|
<optional: implementation details, debugging process, architecture notes>
|
|
```
|
|
|
|
### Rules
|
|
|
|
1. **Subject line**: imperative mood, present tense, 50 characters or less
|
|
- Good: `feat(ai): add AI client with multi-provider support`
|
|
- Bad: `fixed the bug with the ai client that was causing issues`
|
|
|
|
2. **First paragraph**: state WHAT was changed and WHY in 2-3 sentences. No code references.
|
|
|
|
3. **Optional "How" section**: implementation details, debugging process, architecture decisions. This is "extra-credit" reading.
|
|
|
|
4. **Never bury the lead**: if the commit changes one file, the subject line should say what it does. Don't make readers scroll to find out.
|
|
|
|
### Anti-Patterns (from David Thompson's "Favorite Commit")
|
|
|
|
| Anti-Pattern | Example | Fix |
|
|
|---|---|---|
|
|
| Burying the change | 6 paragraphs before showing the diff | Put the summary first |
|
|
| Unexplained problem | "I introduced some tests..." without saying what broke | State the problem explicitly |
|
|
| Unlinked code references | "removing the .with_content() matchers" | Name the file or link to it |
|
|
|
|
### Example
|
|
|
|
```
|
|
feat(ai): add AI client with multi-provider support
|
|
|
|
Add an AI client module that integrates with OpenAI-compatible API
|
|
providers (OpenAI, Perplexity, and custom providers) to provide
|
|
AI-assisted responses within the CProof client.
|
|
|
|
The implementation includes provider management, session handling,
|
|
async HTTP request handling via libcurl, and a dedicated AI window
|
|
for displaying conversations.
|
|
|
|
Architecture:
|
|
- Async design: HTTP requests run on a separate thread to avoid
|
|
blocking the main UI loop
|
|
- Reference counting: Both AIProvider and AISession use ref counting
|
|
for safe shared ownership
|
|
- Response size limit: 10MB cap on HTTP responses to prevent OOM
|
|
```
|
|
|
|
---
|
|
|
|
## 2. GitHub Issues
|
|
|
|
### Structure
|
|
|
|
```markdown
|
|
# <type>: <issue title>
|
|
|
|
## Motivation
|
|
|
|
<Why does this matter? Who benefits?>
|
|
|
|
## Proposed Solution
|
|
|
|
<What should happen? Keep it implementation-agnostic.>
|
|
|
|
## Acceptance Criteria
|
|
|
|
- [ ] AC-1: <verifiable condition>
|
|
- [ ] AC-2: <verifiable condition>
|
|
- [ ] ...
|
|
|
|
## Open Questions
|
|
|
|
<Unresolved decisions that need input>
|
|
|
|
## Related
|
|
|
|
<Links to related issues, docs, or external resources>
|
|
```
|
|
|
|
### Rules
|
|
|
|
1. **Title**: clear, specific, action-oriented. Use conventional commit prefixes where appropriate (`feat:`, `fix:`, `docs:`, etc.)
|
|
|
|
2. **Motivation first**: explain WHY this matters before describing WHAT to build. Focus on user value, not technical details.
|
|
|
|
3. **Acceptance criteria**: verifiable, testable conditions. Use function names or command syntax where helpful, but avoid implementation specifics (no "use GHashTable" or "add this function signature").
|
|
|
|
4. **Keep it general**: describe the feature, not the implementation. The "how" is for the PR/commit, not the issue.
|
|
|
|
5. **Commands over code**: when describing user-facing behavior, use command syntax (`/ai start`) rather than function names (`ai_window_create()`).
|
|
|
|
### Anti-Patterns
|
|
|
|
| Anti-Pattern | Example | Fix |
|
|
|---|---|---|
|
|
| Implementation details in issue | "Add a GHashTable in ai_client.c" | Describe the feature: "Users can manage multiple AI providers" |
|
|
| Vague acceptance criteria | "Make it work" | "Running `/ai start` opens a dedicated AI conversation window" |
|
|
| No motivation | Jumping straight to the solution | Explain WHY this matters to users |
|
|
|
|
### Example
|
|
|
|
```markdown
|
|
# feat(ai): AI Client with Multi-Provider Support
|
|
|
|
## Motivation
|
|
|
|
CProof users want AI-assisted responses without leaving the chat client.
|
|
This feature enables users to query AI models directly from within CProof
|
|
for quick answers, message drafting, or knowledge retrieval.
|
|
|
|
### Privacy First
|
|
|
|
- No telemetry: No data is sent to CProof or any third party except
|
|
the configured AI provider
|
|
- Local-first: Any OpenAI-compatible API endpoint works, including
|
|
local servers (Ollama, LM Studio, vLLM)
|
|
- User-controlled keys: API keys are stored per-provider in the
|
|
preferences file, never hardcoded
|
|
|
|
## Proposed Feature
|
|
|
|
Add an AI client module to CProof that lets users interact with AI
|
|
providers from within the chat interface.
|
|
|
|
A user should be able to:
|
|
1. Start an AI session with `/ai start`
|
|
2. See a dedicated AI window open for the conversation
|
|
3. Send messages and receive AI responses
|
|
4. Switch between providers and models
|
|
5. Use tab-completion for provider names (`/ai s<tab>`)
|
|
6. Have providers and API keys persist across CProof sessions
|
|
|
|
## Acceptance Criteria
|
|
|
|
### Core Functionality
|
|
|
|
- [ ] AC-1: Running `/ai start` opens a dedicated AI conversation window
|
|
- [ ] AC-2: The AI window displays the user's prompt and the AI's response
|
|
- [ ] AC-3: AI responses are properly formatted, including multiline responses
|
|
- [ ] AC-4: The conversation maintains history within a session
|
|
- [ ] AC-5: Running `/ai stop` closes the AI window and ends the session
|
|
|
|
### Provider Management
|
|
|
|
- [ ] AC-6: Default providers are available out of the box
|
|
- [ ] AC-7: Users can add custom providers with `/ai provider add <name> <url>`
|
|
- [ ] AC-8: Users can list available providers with `/ai provider list`
|
|
|
|
### API Key Management
|
|
|
|
- [ ] AC-9: Users can set an API key for a provider with `/ai key <provider> <key>`
|
|
- [ ] AC-10: API keys persist across CProof sessions
|
|
- [ ] AC-11: API keys are not displayed in plain text when listed
|
|
|
|
### Autocomplete
|
|
|
|
- [ ] AC-12: `/ai s<tab>` autocompletes to available providers
|
|
- [ ] AC-13: Autocomplete works for custom providers added by the user
|
|
|
|
### Error Handling
|
|
|
|
- [ ] AC-14: If the API key is invalid, the user sees a clear error message
|
|
- [ ] AC-15: If the provider is unreachable, the user sees a clear error message
|
|
|
|
## Open Questions
|
|
|
|
1. Should we support streaming responses (displaying text as it arrives)?
|
|
2. Should there be a rate limit or timeout for AI responses?
|
|
|
|
## Related
|
|
|
|
- [OpenAI Responses API](https://platform.openai.com/docs/api-reference/responses)
|
|
- [Ollama](https://ollama.com/) (local server)
|
|
```
|
|
|
|
---
|
|
|
|
## 3. Pull Request Descriptions
|
|
|
|
### Structure
|
|
|
|
```markdown
|
|
# <type>: <PR title>
|
|
|
|
## Introduced change
|
|
|
|
<2-3 sentences: what this PR does and why it matters>
|
|
|
|
### Capabilities
|
|
|
|
<Bullet list of key features/capabilities>
|
|
|
|
## Reasoning behind the change
|
|
|
|
<Motivation, design decisions, trade-offs considered>
|
|
|
|
## Implementation details
|
|
|
|
<Architecture overview, key implementation details, code snippets if helpful>
|
|
|
|
## Testing
|
|
|
|
<How to run tests, what was tested>
|
|
|
|
## Screenshots (if UI changes)
|
|
|
|
<Before/after images>
|
|
|
|
Resolves #<issue-number>
|
|
```
|
|
|
|
### Rules
|
|
|
|
1. **"Introduced change"**: the first section should answer "what" without requiring the reader to scroll. Use bullet points for capabilities.
|
|
|
|
2. **"Reasoning behind the change"**: explain design decisions, trade-offs, and alternatives considered. This is where you justify your approach.
|
|
|
|
3. **"Implementation details"**: architecture diagrams, code snippets, data structures. This is "extra-credit" reading for reviewers who want to understand the internals.
|
|
|
|
4. **No git diff stat**: don't include "X files changed, +Y lines". That's visible in the PR UI.
|
|
|
|
5. **Commands over code**: when describing user-facing behavior, use command syntax rather than function names.
|
|
|
|
6. **Implicit sections**: the sections flow naturally — "Introduced change" covers what, "Reasoning" covers why, "Implementation details" covers how. Don't label them explicitly as "What/Why/How".
|
|
|
|
### Anti-Patterns
|
|
|
|
| Anti-Pattern | Example | Fix |
|
|
|---|---|---|
|
|
| Git diff stat | "20 files changed, +2400 insertions" | Remove — visible in PR UI |
|
|
| Implementation in "What" | "Added ai_client.c with 843 lines" | Describe the feature: "Core AI client with provider management" |
|
|
| No "Why" section | Just listing changes | Explain design decisions and trade-offs |
|
|
|
|
### Example
|
|
|
|
```markdown
|
|
# feat(ai): add AI client with multi-provider support and UI
|
|
|
|
## Introduced change
|
|
|
|
An AI client module that integrates with OpenAI-compatible API providers
|
|
to deliver AI-assisted responses within the CProof chat client. Users can
|
|
create AI sessions, send prompts, and view responses in a dedicated AI
|
|
window — all from within the terminal UI.
|
|
|
|
### Capabilities
|
|
|
|
- **Multi-provider support**: Ships with OpenAI and Perplexity as defaults;
|
|
add custom providers via `/ai provider add`
|
|
- **Per-provider API keys**: Each provider's API key is stored in the
|
|
preferences system
|
|
- **Conversation history**: Sessions maintain message history (user/assistant
|
|
turns) for context-aware responses
|
|
- **Async requests**: HTTP calls run on a background thread; responses and
|
|
errors invoke callbacks on the main UI thread
|
|
- **Model selection**: Switch between models per session (e.g., `gpt-4`, `sonar`)
|
|
- **Provider autocomplete**: Tab-completion for provider names in commands
|
|
|
|
## Reasoning behind the change
|
|
|
|
CProof is an XMPP client focused on privacy and usability. Adding AI support
|
|
gives users a way to get quick answers, message drafting assistance, or
|
|
knowledge retrieval without leaving the chat client.
|
|
|
|
The design prioritizes:
|
|
|
|
1. **Privacy**: API keys are stored per-provider in preferences, not hardcoded.
|
|
Users control which providers they use. No data is shared with us, local
|
|
OpenAI-compatible servers are also supported.
|
|
2. **Extensibility**: The provider abstraction (`AIProvider` struct with name,
|
|
URL, org_id) makes it trivial to add new providers.
|
|
3. **Non-blocking UI**: Async HTTP ensures the terminal UI remains responsive
|
|
during API calls.
|
|
4. **Safety**: Response size capped at 10MB; reference counting prevents
|
|
use-after-free on shared objects.
|
|
|
|
## Implementation details
|
|
|
|
### Architecture Overview
|
|
|
|
```
|
|
┌─────────────────────────────────────────────────────────────┐
|
|
│ UI Layer │
|
|
│ ┌──────────────┐ ┌──────────────┐ ┌───────────────────┐ │
|
|
│ │ ProfAiWin │ │ /ai command │ │ Provider autocomplete│ │
|
|
│ │ (window.c) │ │ (cmd_funcs) │ │ (cmd_ac.c) │ │
|
|
│ └──────┬───────┘ └──────┬───────┘ └────────┬──────────┘ │
|
|
│ │ │ │ │
|
|
│ ▼ ▼ │ │
|
|
│ ┌──────────────────────────────────────────────────┐ │
|
|
│ │ AI Client (ai_client.c) │ │
|
|
│ │ ┌────────────┐ ┌────────────┐ ┌───────────┐ │ │
|
|
│ │ │ Providers │ │ Sessions │ │ curl HTTP│ │ │
|
|
│ │ │ (GHashTable)│ │(ref-counted)│ │(async) │ │ │
|
|
│ │ └────────────┘ └────────────┘ └───────────┘ │ │
|
|
│ └──────────────────────────────────────────────────┘ │
|
|
│ │ │
|
|
│ ▼ │
|
|
│ ┌──────────────────────────────────────────────────┐ │
|
|
│ │ Preferences (config/preferences.c) │ │
|
|
│ │ Stores: api_keys[provider_name] │ │
|
|
│ └──────────────────────────────────────────────────┘ │
|
|
└─────────────────────────────────────────────────────────────┘
|
|
```
|
|
|
|
### Key Implementation Details
|
|
|
|
**Provider Management** — Providers are stored in a `GHashTable` keyed by name.
|
|
Each provider has a reference count for safe shared ownership:
|
|
|
|
```c
|
|
typedef struct ai_provider_t {
|
|
gchar* name;
|
|
gchar* api_url;
|
|
gchar* org_id;
|
|
GList* models;
|
|
guint ref_count;
|
|
} AIProvider;
|
|
```
|
|
|
|
Default providers are registered during `ai_client_init()`:
|
|
|
|
| Provider | URL |
|
|
|---|---|
|
|
| `openai` | `https://api.openai.com/v1/responses` |
|
|
| `perplexity` | `https://api.perplexity.ai/v1/responses` |
|
|
|
|
**Session Lifecycle** — Sessions track conversation history and are reference-counted:
|
|
|
|
```c
|
|
typedef struct ai_session_t {
|
|
gchar* provider_name;
|
|
AIProvider* provider;
|
|
gchar* model;
|
|
gchar* api_key;
|
|
GList* history; // GList of AIMessage*
|
|
guint ref_count;
|
|
} AISession;
|
|
```
|
|
|
|
#### Async HTTP Request
|
|
|
|
`ai_send_prompt()` spawns a GThread that:
|
|
1. Builds a JSON body from the session's conversation history
|
|
2. Sends a POST request via libcurl with the provider's API key
|
|
3. Invokes the user's callback on the main thread with the response or error
|
|
|
|
Response size is capped at 10MB to prevent OOM conditions.
|
|
|
|
#### UI Integration
|
|
|
|
A new `ProfAiWin` window type displays AI conversations. Responses are streamed
|
|
line-by-line; errors are displayed in red.
|
|
|
|
#### Testing
|
|
|
|
Unit tests cover:
|
|
- Provider CRUD (add, remove, list, update)
|
|
- Session lifecycle (create, ref/unref, message history, clear)
|
|
- API key get/set
|
|
- JSON string escaping (special chars, backslashes, percent signs)
|
|
- Provider autocomplete (forward, backward, partial match, case sensitivity)
|
|
|
|
Resolves #110
|
|
```
|
|
|
|
---
|
|
|
|
## Quick Reference
|
|
|
|
### Conventional Commits Prefixes
|
|
|
|
| Prefix | Meaning |
|
|
|---|---|
|
|
| `feat` | New feature |
|
|
| `fix` | Bug fix |
|
|
| `docs` | Documentation changes |
|
|
| `style` | Code style changes (formatting, semicolons, etc.) |
|
|
| `refactor` | Code changes that neither fix a bug nor add a feature |
|
|
| `test` | Adding or modifying tests |
|
|
| `chore` | Build process, auxiliary tools, CI changes |
|
|
| `perf` | Performance improvements |
|
|
| `ci` | CI configuration changes |
|
|
| `build` | Build system or dependency changes |
|
|
|
|
### File Naming
|
|
|
|
- CProof (not Profanity) — this is a fork
|
|
- Use snake_case for files: `ai_client.c`, `ai_client.h`
|
|
- Use snake_case for functions: `ai_session_create()`
|
|
- Use PascalCase for types: `AIProvider`, `AISession`
|